Page MenuHomePhabricator

Add documentation illustrating use of IgnoreUnlessSpelledInSource

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

Diff Detail

Event Timeline

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

Thank you for the documentation effort!


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.


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


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.


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


<pre> tags around traverse()


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)?


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


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


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?


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

steveire added inline comments.Thu, Nov 19, 11:45 AM

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

aaron.ballman added inline comments.Thu, Nov 19, 12:45 PM

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?


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

steveire added inline comments.Fri, Nov 20, 2:35 AM

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

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

LGTM, thank you for the new documentation!

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