[RFC] Upstreaming proposed change to .dia format from Swift

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[RFC] Upstreaming proposed change to .dia format from Swift

Fangrui Song via cfe-dev
Hi all,

I was hoping to get some feedback on a proposed change to clang's serialized diagnostics format.

To give some background, Swift (swift.org) also uses this format for its diagnostics, and we recently added a new feature called "educational notes" which we'd like to start serializing. An educational note is attached to a diagnostic and is essentially just a path to associated documentation somewhere in a compiler toolchain. The documentation can then be displayed alongside emitted diagnostics to teach users about relevant language concepts or describe common problems and solutions. More details can be found at https://github.com/apple/swift/blob/master/docs/Diagnostics.md#educational-notes and an example is available at https://github.com/apple/swift/blob/master/userdocs/diagnostics/temporary-pointers.md

In order to support this use case, I'd like to propose adding new records to the .dia format for educational note paths, or perhaps more generally, documentation paths. The new records would be emitted alongside the main diagnostic record, similar to the handling of ranges and fix-its. I've submitted a patch with the proposed changes here: https://reviews.llvm.org/D80126 .

The main reason we'd like to upstream this change to Clang instead of just implementing it in Swift is to avoid diverging from the established format. However, it isn't language specific and in the long term I think this is a feature that could benefit Clang's diagnostics quite a bit as well.

My questions are:
  • Is the clang community open to making a change like this that primarily benefits a downstream project (swift) in the short term?
  • Is this a feature clang might be interested in adopting at some point? 

Any and all feedback is welcome! Sorry if this is the wrong place to discuss this sort of thing — it's my first time posting to the LLVM lists.

Thanks,
Owen

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Upstreaming proposed change to .dia format from Swift

Fangrui Song via cfe-dev
Hi, Owen,

I think having documentation links associated with diagnostics is a useful feature, and I would be happy to see it in Clang.

I would encourage the addition of appropriate links to Clang's https://clang.llvm.org/docs/LanguageExtensions.html and, moreover, maybe we can generate these for mis-applied attributes in some automated way (given that our attribute docs are also TableGen-generated).

 -Hal

Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory


From: cfe-dev <[hidden email]> on behalf of Owen Voorhees via cfe-dev <[hidden email]>
Sent: Monday, May 18, 2020 10:15 AM
To: [hidden email] <[hidden email]>
Subject: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
 
Hi all,

I was hoping to get some feedback on a proposed change to clang's serialized diagnostics format.

To give some background, Swift (swift.org) also uses this format for its diagnostics, and we recently added a new feature called "educational notes" which we'd like to start serializing. An educational note is attached to a diagnostic and is essentially just a path to associated documentation somewhere in a compiler toolchain. The documentation can then be displayed alongside emitted diagnostics to teach users about relevant language concepts or describe common problems and solutions. More details can be found at https://github.com/apple/swift/blob/master/docs/Diagnostics.md#educational-notes and an example is available at https://github.com/apple/swift/blob/master/userdocs/diagnostics/temporary-pointers.md

In order to support this use case, I'd like to propose adding new records to the .dia format for educational note paths, or perhaps more generally, documentation paths. The new records would be emitted alongside the main diagnostic record, similar to the handling of ranges and fix-its. I've submitted a patch with the proposed changes here: https://reviews.llvm.org/D80126 .

The main reason we'd like to upstream this change to Clang instead of just implementing it in Swift is to avoid diverging from the established format. However, it isn't language specific and in the long term I think this is a feature that could benefit Clang's diagnostics quite a bit as well.

My questions are:
  • Is the clang community open to making a change like this that primarily benefits a downstream project (swift) in the short term?
  • Is this a feature clang might be interested in adopting at some point? 

Any and all feedback is welcome! Sorry if this is the wrong place to discuss this sort of thing — it's my first time posting to the LLVM lists.

Thanks,
Owen

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Upstreaming proposed change to .dia format from Swift

Fangrui Song via cfe-dev
On Tue, May 19, 2020 at 6:26 AM Finkel, Hal J. <[hidden email]> wrote:
>
> Hi, Owen,
>
> I think having documentation links associated with diagnostics is a useful feature, and I would be happy to see it in Clang.

I agree, I think this is a great idea! Hopefully we can also make use
of it (or something like it) in clang-tidy as well.

> I would encourage the addition of appropriate links to Clang's https://clang.llvm.org/docs/LanguageExtensions.html and, moreover, maybe we can generate these for mis-applied attributes in some automated way (given that our attribute docs are also TableGen-generated).

Are you envisioning something like `[[gnu::nonnull]] int *i;` giving
you a diagnostic about the attribute being ignored because it doesn't
apply to local variables, along with a link directly to the
documentation for that attribute? If so, that's a neat idea! We could
possibly even do something like that for unknown or misspelled command
line arguments, because the command line reference is also table
generated.

~Aaron

>
>  -Hal
>
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
> ________________________________
> From: cfe-dev <[hidden email]> on behalf of Owen Voorhees via cfe-dev <[hidden email]>
> Sent: Monday, May 18, 2020 10:15 AM
> To: [hidden email] <[hidden email]>
> Subject: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
>
> Hi all,
>
> I was hoping to get some feedback on a proposed change to clang's serialized diagnostics format.
>
> To give some background, Swift (swift.org) also uses this format for its diagnostics, and we recently added a new feature called "educational notes" which we'd like to start serializing. An educational note is attached to a diagnostic and is essentially just a path to associated documentation somewhere in a compiler toolchain. The documentation can then be displayed alongside emitted diagnostics to teach users about relevant language concepts or describe common problems and solutions. More details can be found at https://github.com/apple/swift/blob/master/docs/Diagnostics.md#educational-notes and an example is available at https://github.com/apple/swift/blob/master/userdocs/diagnostics/temporary-pointers.md.
>
> In order to support this use case, I'd like to propose adding new records to the .dia format for educational note paths, or perhaps more generally, documentation paths. The new records would be emitted alongside the main diagnostic record, similar to the handling of ranges and fix-its. I've submitted a patch with the proposed changes here: https://reviews.llvm.org/D80126 .
>
> The main reason we'd like to upstream this change to Clang instead of just implementing it in Swift is to avoid diverging from the established format. However, it isn't language specific and in the long term I think this is a feature that could benefit Clang's diagnostics quite a bit as well.
>
> My questions are:
>
> Is the clang community open to making a change like this that primarily benefits a downstream project (swift) in the short term?
> Is this a feature clang might be interested in adopting at some point?
>
>
> Any and all feedback is welcome! Sorry if this is the wrong place to discuss this sort of thing — it's my first time posting to the LLVM lists.
>
> Thanks,
> Owen
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Upstreaming proposed change to .dia format from Swift

Fangrui Song via cfe-dev


From: Aaron Ballman <[hidden email]>
Sent: Tuesday, May 19, 2020 7:44 AM
To: Finkel, Hal J. <[hidden email]>
Cc: [hidden email] <[hidden email]>; Owen Voorhees <[hidden email]>
Subject: Re: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
 
On Tue, May 19, 2020 at 6:26 AM Finkel, Hal J. <[hidden email]> wrote:
>
> Hi, Owen,
>
> I think having documentation links associated with diagnostics is a useful feature, and I would be happy to see it in Clang.

I agree, I think this is a great idea! Hopefully we can also make use
of it (or something like it) in clang-tidy as well.

> I would encourage the addition of appropriate links to Clang's https://clang.llvm.org/docs/LanguageExtensions.html and, moreover, maybe we can generate these for mis-applied attributes in some automated way (given that our attribute docs are also TableGen-generated).

Are you envisioning something like `[[gnu::nonnull]] int *i;` giving
you a diagnostic about the attribute being ignored because it doesn't
apply to local variables, along with a link directly to the
documentation for that attribute? If so, that's a neat idea! We could
possibly even do something like that for unknown or misspelled command
line arguments, because the command line reference is also table
generated.


Yes, exactly. I'm glad that you agree 🙂

 -Hal


~Aaron

>
>  -Hal
>
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
> ________________________________
> From: cfe-dev <[hidden email]> on behalf of Owen Voorhees via cfe-dev <[hidden email]>
> Sent: Monday, May 18, 2020 10:15 AM
> To: [hidden email] <[hidden email]>
> Subject: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
>
> Hi all,
>
> I was hoping to get some feedback on a proposed change to clang's serialized diagnostics format.
>
> To give some background, Swift (swift.org) also uses this format for its diagnostics, and we recently added a new feature called "educational notes" which we'd like to start serializing. An educational note is attached to a diagnostic and is essentially just a path to associated documentation somewhere in a compiler toolchain. The documentation can then be displayed alongside emitted diagnostics to teach users about relevant language concepts or describe common problems and solutions. More details can be found at https://github.com/apple/swift/blob/master/docs/Diagnostics.md#educational-notes and an example is available at https://github.com/apple/swift/blob/master/userdocs/diagnostics/temporary-pointers.md.
>
> In order to support this use case, I'd like to propose adding new records to the .dia format for educational note paths, or perhaps more generally, documentation paths. The new records would be emitted alongside the main diagnostic record, similar to the handling of ranges and fix-its. I've submitted a patch with the proposed changes here: https://reviews.llvm.org/D80126 .
>
> The main reason we'd like to upstream this change to Clang instead of just implementing it in Swift is to avoid diverging from the established format. However, it isn't language specific and in the long term I think this is a feature that could benefit Clang's diagnostics quite a bit as well.
>
> My questions are:
>
> Is the clang community open to making a change like this that primarily benefits a downstream project (swift) in the short term?
> Is this a feature clang might be interested in adopting at some point?
>
>
> Any and all feedback is welcome! Sorry if this is the wrong place to discuss this sort of thing — it's my first time posting to the LLVM lists.
>
> Thanks,
> Owen

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Upstreaming proposed change to .dia format from Swift

Fangrui Song via cfe-dev
Thanks for pointing me towards those docs, they seem like excellent candidates for linking from diagnostics! Providing more information when encountering errors related to things like attributes and command line args that users may not encounter on a regular basis is exactly in line with my goals when adding this.

It seems apparent that I probably shouldn’t assume documentation will be installed locally when generalizing this feature to work with Clang. Swift ships the docs alongside the compiler primarily because the language and diagnostics are still evolving rapidly, and we don’t have a good mechanism to host versioned documentation at the moment. On the other hand, linking to something like https://clang.llvm.org/docs/LanguageExtensions.html#non-standard-c-11-attributes from Clang diagnostics seems perfectly sensible, and there’s no reason I can think of not to support both web and file URLs.

I’ll look into Clang’s diagnostics infrastructure to get a better idea of how documentation links can be integrated throughout before moving ahead with the serialization format updates. I think my originally proposed changes are still a little too Swift-specific.

— Owen

On May 19, 2020, at 7:49 AM, Finkel, Hal J. <[hidden email]> wrote:



From: Aaron Ballman <[hidden email]>
Sent: Tuesday, May 19, 2020 7:44 AM
To: Finkel, Hal J. <[hidden email]>
Cc: [hidden email] <[hidden email]>; Owen Voorhees <[hidden email]>
Subject: Re: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
 
On Tue, May 19, 2020 at 6:26 AM Finkel, Hal J. <[hidden email]> wrote:
>
> Hi, Owen,
>
> I think having documentation links associated with diagnostics is a useful feature, and I would be happy to see it in Clang.

I agree, I think this is a great idea! Hopefully we can also make use
of it (or something like it) in clang-tidy as well.

> I would encourage the addition of appropriate links to Clang's https://clang.llvm.org/docs/LanguageExtensions.html and, moreover, maybe we can generate these for mis-applied attributes in some automated way (given that our attribute docs are also TableGen-generated).

Are you envisioning something like `[[gnu::nonnull]] int *i;` giving
you a diagnostic about the attribute being ignored because it doesn't
apply to local variables, along with a link directly to the
documentation for that attribute? If so, that's a neat idea! We could
possibly even do something like that for unknown or misspelled command
line arguments, because the command line reference is also table
generated.


Yes, exactly. I'm glad that you agree 🙂

 -Hal


~Aaron

>
>  -Hal
>
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
> ________________________________
> From: cfe-dev <[hidden email]> on behalf of Owen Voorhees via cfe-dev <[hidden email]>
> Sent: Monday, May 18, 2020 10:15 AM
> To: [hidden email] <[hidden email]>
> Subject: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
>
> Hi all,
>
> I was hoping to get some feedback on a proposed change to clang's serialized diagnostics format.
>
> To give some background, Swift (swift.org) also uses this format for its diagnostics, and we recently added a new feature called "educational notes" which we'd like to start serializing. An educational note is attached to a diagnostic and is essentially just a path to associated documentation somewhere in a compiler toolchain. The documentation can then be displayed alongside emitted diagnostics to teach users about relevant language concepts or describe common problems and solutions. More details can be found at https://github.com/apple/swift/blob/master/docs/Diagnostics.md#educational-notes and an example is available at https://github.com/apple/swift/blob/master/userdocs/diagnostics/temporary-pointers.md.
>
> In order to support this use case, I'd like to propose adding new records to the .dia format for educational note paths, or perhaps more generally, documentation paths. The new records would be emitted alongside the main diagnostic record, similar to the handling of ranges and fix-its. I've submitted a patch with the proposed changes here: https://reviews.llvm.org/D80126 .
>
> The main reason we'd like to upstream this change to Clang instead of just implementing it in Swift is to avoid diverging from the established format. However, it isn't language specific and in the long term I think this is a feature that could benefit Clang's diagnostics quite a bit as well.
>
> My questions are:
>
> Is the clang community open to making a change like this that primarily benefits a downstream project (swift) in the short term?
> Is this a feature clang might be interested in adopting at some point?
>
>
> Any and all feedback is welcome! Sorry if this is the wrong place to discuss this sort of thing — it's my first time posting to the LLVM lists.
>
> Thanks,
> Owen

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Upstreaming proposed change to .dia format from Swift

Fangrui Song via cfe-dev
Hi Owen,

I think this is a great idea!

I'm just thinking about how to make sure the URLs in diagnostics are valid and stay that way.

We might want to use a combination of CI jobs and phabricator checks to make sure all the URLs we produce in diagnostics in both current master and somewhat recent releases are ok.


Jan

On May 19, 2020, at 9:14 AM, Owen Voorhees via cfe-dev <[hidden email]> wrote:

Thanks for pointing me towards those docs, they seem like excellent candidates for linking from diagnostics! Providing more information when encountering errors related to things like attributes and command line args that users may not encounter on a regular basis is exactly in line with my goals when adding this.

It seems apparent that I probably shouldn’t assume documentation will be installed locally when generalizing this feature to work with Clang. Swift ships the docs alongside the compiler primarily because the language and diagnostics are still evolving rapidly, and we don’t have a good mechanism to host versioned documentation at the moment. On the other hand, linking to something like https://clang.llvm.org/docs/LanguageExtensions.html#non-standard-c-11-attributes from Clang diagnostics seems perfectly sensible, and there’s no reason I can think of not to support both web and file URLs.

I’ll look into Clang’s diagnostics infrastructure to get a better idea of how documentation links can be integrated throughout before moving ahead with the serialization format updates. I think my originally proposed changes are still a little too Swift-specific.

— Owen

On May 19, 2020, at 7:49 AM, Finkel, Hal J. <[hidden email]> wrote:



From: Aaron Ballman <[hidden email]>
Sent: Tuesday, May 19, 2020 7:44 AM
To: Finkel, Hal J. <[hidden email]>
Cc: [hidden email] <[hidden email]>; Owen Voorhees <[hidden email]>
Subject: Re: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
 
On Tue, May 19, 2020 at 6:26 AM Finkel, Hal J. <[hidden email]> wrote:
>
> Hi, Owen,
>
> I think having documentation links associated with diagnostics is a useful feature, and I would be happy to see it in Clang.

I agree, I think this is a great idea! Hopefully we can also make use
of it (or something like it) in clang-tidy as well.

> I would encourage the addition of appropriate links to Clang's https://clang.llvm.org/docs/LanguageExtensions.html and, moreover, maybe we can generate these for mis-applied attributes in some automated way (given that our attribute docs are also TableGen-generated).

Are you envisioning something like `[[gnu::nonnull]] int *i;` giving
you a diagnostic about the attribute being ignored because it doesn't
apply to local variables, along with a link directly to the
documentation for that attribute? If so, that's a neat idea! We could
possibly even do something like that for unknown or misspelled command
line arguments, because the command line reference is also table
generated.


Yes, exactly. I'm glad that you agree 🙂

 -Hal


~Aaron

>
>  -Hal
>
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
> ________________________________
> From: cfe-dev <[hidden email]> on behalf of Owen Voorhees via cfe-dev <[hidden email]>
> Sent: Monday, May 18, 2020 10:15 AM
> To: [hidden email] <[hidden email]>
> Subject: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
>
> Hi all,
>
> I was hoping to get some feedback on a proposed change to clang's serialized diagnostics format.
>
> To give some background, Swift (swift.org) also uses this format for its diagnostics, and we recently added a new feature called "educational notes" which we'd like to start serializing. An educational note is attached to a diagnostic and is essentially just a path to associated documentation somewhere in a compiler toolchain. The documentation can then be displayed alongside emitted diagnostics to teach users about relevant language concepts or describe common problems and solutions. More details can be found at https://github.com/apple/swift/blob/master/docs/Diagnostics.md#educational-notes and an example is available at https://github.com/apple/swift/blob/master/userdocs/diagnostics/temporary-pointers.md.
>
> In order to support this use case, I'd like to propose adding new records to the .dia format for educational note paths, or perhaps more generally, documentation paths. The new records would be emitted alongside the main diagnostic record, similar to the handling of ranges and fix-its. I've submitted a patch with the proposed changes here: https://reviews.llvm.org/D80126 .
>
> The main reason we'd like to upstream this change to Clang instead of just implementing it in Swift is to avoid diverging from the established format. However, it isn't language specific and in the long term I think this is a feature that could benefit Clang's diagnostics quite a bit as well.
>
> My questions are:
>
> Is the clang community open to making a change like this that primarily benefits a downstream project (swift) in the short term?
> Is this a feature clang might be interested in adopting at some point?
>
>
> Any and all feedback is welcome! Sorry if this is the wrong place to discuss this sort of thing — it's my first time posting to the LLVM lists.
>
> Thanks,
> Owen
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Upstreaming proposed change to .dia format from Swift

Fangrui Song via cfe-dev
Might be worth passing all URLs through some central (well, not central-central - obviously Swift wants to manage its own separate from Clang and the LLVM community) URL forwarding thing we can update more reliably (eg: what if we decide to rename a URL, use a different file format (so the extension changes from '.rst'), etc?)

On Wed, May 20, 2020 at 1:42 PM Jan Korous via cfe-dev <[hidden email]> wrote:
Hi Owen,

I think this is a great idea!

I'm just thinking about how to make sure the URLs in diagnostics are valid and stay that way.

We might want to use a combination of CI jobs and phabricator checks to make sure all the URLs we produce in diagnostics in both current master and somewhat recent releases are ok.


Jan

On May 19, 2020, at 9:14 AM, Owen Voorhees via cfe-dev <[hidden email]> wrote:

Thanks for pointing me towards those docs, they seem like excellent candidates for linking from diagnostics! Providing more information when encountering errors related to things like attributes and command line args that users may not encounter on a regular basis is exactly in line with my goals when adding this.

It seems apparent that I probably shouldn’t assume documentation will be installed locally when generalizing this feature to work with Clang. Swift ships the docs alongside the compiler primarily because the language and diagnostics are still evolving rapidly, and we don’t have a good mechanism to host versioned documentation at the moment. On the other hand, linking to something like https://clang.llvm.org/docs/LanguageExtensions.html#non-standard-c-11-attributes from Clang diagnostics seems perfectly sensible, and there’s no reason I can think of not to support both web and file URLs.

I’ll look into Clang’s diagnostics infrastructure to get a better idea of how documentation links can be integrated throughout before moving ahead with the serialization format updates. I think my originally proposed changes are still a little too Swift-specific.

— Owen

On May 19, 2020, at 7:49 AM, Finkel, Hal J. <[hidden email]> wrote:



From: Aaron Ballman <[hidden email]>
Sent: Tuesday, May 19, 2020 7:44 AM
To: Finkel, Hal J. <[hidden email]>
Cc: [hidden email] <[hidden email]>; Owen Voorhees <[hidden email]>
Subject: Re: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
 
On Tue, May 19, 2020 at 6:26 AM Finkel, Hal J. <[hidden email]> wrote:
>
> Hi, Owen,
>
> I think having documentation links associated with diagnostics is a useful feature, and I would be happy to see it in Clang.

I agree, I think this is a great idea! Hopefully we can also make use
of it (or something like it) in clang-tidy as well.

> I would encourage the addition of appropriate links to Clang's https://clang.llvm.org/docs/LanguageExtensions.html and, moreover, maybe we can generate these for mis-applied attributes in some automated way (given that our attribute docs are also TableGen-generated).

Are you envisioning something like `[[gnu::nonnull]] int *i;` giving
you a diagnostic about the attribute being ignored because it doesn't
apply to local variables, along with a link directly to the
documentation for that attribute? If so, that's a neat idea! We could
possibly even do something like that for unknown or misspelled command
line arguments, because the command line reference is also table
generated.


Yes, exactly. I'm glad that you agree 🙂

 -Hal


~Aaron

>
>  -Hal
>
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
> ________________________________
> From: cfe-dev <[hidden email]> on behalf of Owen Voorhees via cfe-dev <[hidden email]>
> Sent: Monday, May 18, 2020 10:15 AM
> To: [hidden email] <[hidden email]>
> Subject: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
>
> Hi all,
>
> I was hoping to get some feedback on a proposed change to clang's serialized diagnostics format.
>
> To give some background, Swift (swift.org) also uses this format for its diagnostics, and we recently added a new feature called "educational notes" which we'd like to start serializing. An educational note is attached to a diagnostic and is essentially just a path to associated documentation somewhere in a compiler toolchain. The documentation can then be displayed alongside emitted diagnostics to teach users about relevant language concepts or describe common problems and solutions. More details can be found at https://github.com/apple/swift/blob/master/docs/Diagnostics.md#educational-notes and an example is available at https://github.com/apple/swift/blob/master/userdocs/diagnostics/temporary-pointers.md.
>
> In order to support this use case, I'd like to propose adding new records to the .dia format for educational note paths, or perhaps more generally, documentation paths. The new records would be emitted alongside the main diagnostic record, similar to the handling of ranges and fix-its. I've submitted a patch with the proposed changes here: https://reviews.llvm.org/D80126 .
>
> The main reason we'd like to upstream this change to Clang instead of just implementing it in Swift is to avoid diverging from the established format. However, it isn't language specific and in the long term I think this is a feature that could benefit Clang's diagnostics quite a bit as well.
>
> My questions are:
>
> Is the clang community open to making a change like this that primarily benefits a downstream project (swift) in the short term?
> Is this a feature clang might be interested in adopting at some point?
>
>
> Any and all feedback is welcome! Sorry if this is the wrong place to discuss this sort of thing — it's my first time posting to the LLVM lists.
>
> Thanks,
> Owen
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] Upstreaming proposed change to .dia format from Swift

Fangrui Song via cfe-dev
Yes, I think if and when web URLs start shipping attached to diagnostics it’s important to have some kind of system in place to check for outdated links. Since URLs will likely be associated with diagnostic IDs using TableGen or some other structured format, I don’t think it would be too difficult to put together a tool to scan for them and check they’re still working.

To give some context, right now Swift exclusively links to documentation installed locally from its diagnostics, and I expect it to stay that way for the foreseeable future. Part of the reason for that was to avoid this exact problem.

However, I think it’s important to support any URL in the serialized format because this approach of local-only diagnostic documentation won’t necessarily adapt well to Clang (and any other compilers that use the format, though I don’t know of any). It worked well for Swift primarily because we had little-to-no existing diagnostics-related documentation. Because everything is being written from scratch, it’s been easy to enforce the requirement that it’s all organized in a central location in the toolchain, and it’s all easy to read as plaintext. On the other hand, Clang already has lots of really great documentation designed for the web, and restructuring it to ship alongside the compiler seems less likely to work well.

The way I see it, allowing any URL to appear in the serialized format gives Clang and Swift the flexibility to choose a different set of tradeoffs based on their different approaches to docs, toolchain layout, etc.. In the end, the reader of the format doesn’t need to worry about these differences, because it’s the compiler’s responsibility to produce valid links, whether that’s through shipping docs locally, enforcing it with CI jobs, or something else entirely.

- Owen



On May 20, 2020, at 4:04 PM, David Blaikie <[hidden email]> wrote:

Might be worth passing all URLs through some central (well, not central-central - obviously Swift wants to manage its own separate from Clang and the LLVM community) URL forwarding thing we can update more reliably (eg: what if we decide to rename a URL, use a different file format (so the extension changes from '.rst'), etc?)

On Wed, May 20, 2020 at 1:42 PM Jan Korous via cfe-dev <[hidden email]> wrote:
Hi Owen,

I think this is a great idea!

I'm just thinking about how to make sure the URLs in diagnostics are valid and stay that way.

We might want to use a combination of CI jobs and phabricator checks to make sure all the URLs we produce in diagnostics in both current master and somewhat recent releases are ok.


Jan

On May 19, 2020, at 9:14 AM, Owen Voorhees via cfe-dev <[hidden email]> wrote:

Thanks for pointing me towards those docs, they seem like excellent candidates for linking from diagnostics! Providing more information when encountering errors related to things like attributes and command line args that users may not encounter on a regular basis is exactly in line with my goals when adding this.

It seems apparent that I probably shouldn’t assume documentation will be installed locally when generalizing this feature to work with Clang. Swift ships the docs alongside the compiler primarily because the language and diagnostics are still evolving rapidly, and we don’t have a good mechanism to host versioned documentation at the moment. On the other hand, linking to something like https://clang.llvm.org/docs/LanguageExtensions.html#non-standard-c-11-attributes from Clang diagnostics seems perfectly sensible, and there’s no reason I can think of not to support both web and file URLs.

I’ll look into Clang’s diagnostics infrastructure to get a better idea of how documentation links can be integrated throughout before moving ahead with the serialization format updates. I think my originally proposed changes are still a little too Swift-specific.

— Owen

On May 19, 2020, at 7:49 AM, Finkel, Hal J. <[hidden email]> wrote:



From: Aaron Ballman <[hidden email]>
Sent: Tuesday, May 19, 2020 7:44 AM
To: Finkel, Hal J. <[hidden email]>
Cc: [hidden email] <[hidden email]>; Owen Voorhees <[hidden email]>
Subject: Re: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
 
On Tue, May 19, 2020 at 6:26 AM Finkel, Hal J. <[hidden email]> wrote:
>
> Hi, Owen,
>
> I think having documentation links associated with diagnostics is a useful feature, and I would be happy to see it in Clang.

I agree, I think this is a great idea! Hopefully we can also make use
of it (or something like it) in clang-tidy as well.

> I would encourage the addition of appropriate links to Clang's https://clang.llvm.org/docs/LanguageExtensions.html and, moreover, maybe we can generate these for mis-applied attributes in some automated way (given that our attribute docs are also TableGen-generated).

Are you envisioning something like `[[gnu::nonnull]] int *i;` giving
you a diagnostic about the attribute being ignored because it doesn't
apply to local variables, along with a link directly to the
documentation for that attribute? If so, that's a neat idea! We could
possibly even do something like that for unknown or misspelled command
line arguments, because the command line reference is also table
generated.


Yes, exactly. I'm glad that you agree 🙂

 -Hal


~Aaron

>
>  -Hal
>
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
> ________________________________
> From: cfe-dev <[hidden email]> on behalf of Owen Voorhees via cfe-dev <[hidden email]>
> Sent: Monday, May 18, 2020 10:15 AM
> To: [hidden email] <[hidden email]>
> Subject: [cfe-dev] [RFC] Upstreaming proposed change to .dia format from Swift
>
> Hi all,
>
> I was hoping to get some feedback on a proposed change to clang's serialized diagnostics format.
>
> To give some background, Swift (swift.org) also uses this format for its diagnostics, and we recently added a new feature called "educational notes" which we'd like to start serializing. An educational note is attached to a diagnostic and is essentially just a path to associated documentation somewhere in a compiler toolchain. The documentation can then be displayed alongside emitted diagnostics to teach users about relevant language concepts or describe common problems and solutions. More details can be found at https://github.com/apple/swift/blob/master/docs/Diagnostics.md#educational-notes and an example is available at https://github.com/apple/swift/blob/master/userdocs/diagnostics/temporary-pointers.md.
>
> In order to support this use case, I'd like to propose adding new records to the .dia format for educational note paths, or perhaps more generally, documentation paths. The new records would be emitted alongside the main diagnostic record, similar to the handling of ranges and fix-its. I've submitted a patch with the proposed changes here: https://reviews.llvm.org/D80126 .
>
> The main reason we'd like to upstream this change to Clang instead of just implementing it in Swift is to avoid diverging from the established format. However, it isn't language specific and in the long term I think this is a feature that could benefit Clang's diagnostics quite a bit as well.
>
> My questions are:
>
> Is the clang community open to making a change like this that primarily benefits a downstream project (swift) in the short term?
> Is this a feature clang might be interested in adopting at some point?
>
>
> Any and all feedback is welcome! Sorry if this is the wrong place to discuss this sort of thing — it's my first time posting to the LLVM lists.
>
> Thanks,
> Owen
_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev