Page MenuHomePhabricator

Add documentation URL records to the .dia format and expose them via libclang
Needs ReviewPublic

Authored by owenv on May 18 2020, 8:10 AM.



A documentation URL is attached to a diagnostic and links to local or remote documentation. The
documentation can then be displayed alongside emitted diagnostics to teach users
about relevant language concepts or describe common problems and solutions.

This commit adds records for serializing documentation URLs in .dia files and
adds support for retrieving them from deserialized diagnostics via libclang.
It does not add any mechanisms for emitting documentation URLs alongside Clang
diagnostics. For now, only downstream projects (swift) support diagnostics
with documentation URLs.

This change is intended to support Swift's "educational notes" feature
as well as future improvements to Clang's diagnostics. A brief discussion of
the goal behind this change can be found here:

Diff Detail

Event Timeline

owenv created this revision.May 18 2020, 8:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 18 2020, 8:10 AM
owenv updated this revision to Diff 265294.May 20 2020, 10:12 AM

Updated terminology to generalize the feature so Clang diagnostics can adopt it in the future.
Also switched from local paths to URLs so local and remote documentation can both be supported if desired

owenv retitled this revision from Add educational note records to the .dia format and expose them via libclang to Add documentation URL records to the .dia format and expose them via libclang.May 20 2020, 10:17 AM
owenv edited the summary of this revision. (Show Details)
owenv added reviewers: aprantl, gribozavr, doug.gregor.

Reviewers added are a best guess based on arc cover. Sorry if I made a mistake, it's my first time contributing to LLVM!

gribozavr2 added a subscriber: gribozavr2.

+ Argyrios for libclang changes.

Hi Owen,
Do you plan to land the functionality for emitting documentation URLs in clang too?


I am just wondering what happens with the ID. Shouldn't we pass it too?

4 ↗(On Diff #265294)

Tip - you can use regexes in CHECKs.

I think we should change this to something like:

// CHECK: [Documentation URL: {{.*}}/doc/swift/diagnostics/]
owenv updated this revision to Diff 265592.May 21 2020, 1:44 PM
  • Update a few comments
  • Simplify test case
owenv marked an inline comment as done.May 21 2020, 1:54 PM

@jkorous I'd like to add support for urls in clang-emitted diagnostics too, but I can't really commit to a timeframe for getting that done atm. I'm hoping to have some time to work on it in the next few weeks.


I think the ID should be Record[0] in the call t visitDocumentationURLRecord below. I based this on the handling of category records.

owenv marked an inline comment as done.May 21 2020, 1:55 PM
owenv added inline comments.

Sorry, I don't know how to reply without marking this as done

jkorous added inline comments.May 22 2020, 10:21 AM

Got it!