This is an archive of the discontinued LLVM Phabricator instance.

Add documentation illustrating use of IgnoreUnlessSpelledInSource
ClosedPublic

Authored by steveire on Nov 17 2020, 9:08 AM.

Diff Detail

Event Timeline

steveire created this revision.Nov 17 2020, 9:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 17 2020, 9:08 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
steveire requested review of this revision.Nov 17 2020, 9:08 AM

Thank you for the documentation effort!

clang/docs/LibASTMatchersReference.html
535

I think this section should go above the Node Matchers section in the document so that Node Matchers stays directly above the table of node matchers.

539

Can you add <pre> tags around AsIs and IgnoreUnlessSpelledInSource to make it clear that these are sort of syntactic constructs rather than prose?

540–542

Rather than say the default mode is hard to use correctly, how about:

This mode requires writing AST matchers that explicitly traverse or ignore implicit nodes, such as parentheses surrounding an expression or expressions with cleanups. These implicit nodes are not always obvious from the syntax of the source code, and so this mode requires careful consideration and testing to get the desired behavior from an AST matcher.

551

familiar with the AST -> familiar with where implicit nodes appear in the AST

563

<pre> tags around traverse()

588

Should we consistently switch this style to:

struct B {

to save on some vertical whitespace (it doesn't impact all of the rows, but will help in some cases)?

723

How about switching the exclamation points into full stops (here and elsewhere)?

738

Backticks won't help here -- should probably use <pre> tags.

747–753

Some more potential places for vertical whitespace savings.

Actually, would it be too onerous to ask you to run all of the code snippets through clang-format with the LLVM style so that they're consistently formatted and trims some of the vertical whitespace?

791

Could use some <pre> tags around the code constructs.

steveire added inline comments.Nov 19 2020, 11:45 AM
clang/docs/LibASTMatchersReference.html
738

pre is equivalent to three backticks. I added a span for use within paragraphs.

aaron.ballman added inline comments.Nov 19 2020, 12:45 PM
clang/docs/LibASTMatchersReference.html
91–94

I was trying to reword this to avoid making a judgmental statement that the default mode is hard to use correctly -- I'd strike this part of the paragraph. WDYT?

738

Thank you for translating my suggestion into the one I was trying to make. :-)

steveire added inline comments.Nov 20 2020, 2:35 AM
clang/docs/LibASTMatchersReference.html
91–94

Oops, that's what I was trying to do, but accidentally left the previous stuff behind. Gone now.

aaron.ballman accepted this revision.Nov 20 2020, 5:45 AM

LGTM, thank you for the new documentation!

This revision is now accepted and ready to land.Nov 20 2020, 5:45 AM
This revision was landed with ongoing or failed builds.Nov 20 2020, 5:50 AM
This revision was automatically updated to reflect the committed changes.