aaron.ballman added a comment.
Thank you for the documentation effort!
================
Comment at: clang/docs/LibASTMatchersReference.html:100
+<!-- =======================================================================
-->
+<h2 id="traverse-mode">Traverse Mode</h2>
+<!-- =======================================================================
-->
----------------
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.
================
Comment at: clang/docs/LibASTMatchersReference.html:104
+<p>The default mode of operation of AST Matchers visits all nodes in the AST,
+even if they are not spelled in the source. This is AsIs mode. This mode is
+hard to use correctly and requires more development iteration because it means
----------------
Can you add `<pre>` tags around `AsIs` and `IgnoreUnlessSpelledInSource` to
make it clear that these are sort of syntactic constructs rather than prose?
================
Comment at: clang/docs/LibASTMatchersReference.html:105-107
+hard to use correctly and requires more development iteration because it means
+that the user must write AST Matchers to explicitly traverse or ignore nodes
+which are not visible.</p>
----------------
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.`
================
Comment at: clang/docs/LibASTMatchersReference.html:116
+the source using the IgnoreUnlessSpelledInSource mode. This is likely to be far
+less error-prone for users who are not already very familiar with the AST. It
is
+also likely to be less error-prone for experienced AST users, as difficult
cases
----------------
familiar with the AST -> familiar with where implicit nodes appear in the AST
================
Comment at: clang/docs/LibASTMatchersReference.html:128
+
+<p>When using the C++ API such as in clang-tidy checks, the traverse() matcher
+is used to set the mode:
----------------
<pre> tags around `traverse()`
================
Comment at: clang/docs/LibASTMatchersReference.html:153
+<pre>
+struct B
+{
----------------
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)?
================
Comment at: clang/docs/LibASTMatchersReference.html:288
+<td>
+Match found! Insertion produces incorrect output:
+<pre>
----------------
How about switching the exclamation points into full stops (here and elsewhere)?
================
Comment at: clang/docs/LibASTMatchersReference.html:303
+<tr>
+ <td>Replacement of `begin()` with `cbegin()`:
+<pre>
----------------
Backticks won't help here -- should probably use <pre> tags.
================
Comment at: clang/docs/LibASTMatchersReference.html:312-318
+void foo()
+{
+ const Container c;
+ c.begin();
+
+ for (auto i : c)
+ {
----------------
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?
================
Comment at: clang/docs/LibASTMatchersReference.html:356
+<tr>
+ <td>Replacement of int member with safe_int:
+<pre>
----------------
Could use some <pre> tags around the code constructs.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91639/new/
https://reviews.llvm.org/D91639
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
