Add documentation illustrating use of IgnoreUnlessSpelledInSource

[steveire]

Update

[aaron.ballman]

Thank you for the documentation effort!

clang/docs/LibASTMatchersReference.html

100

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.

104

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

105–107

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.

116

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

128

<pre> tags around traverse()

153

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

288

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

303

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

312–318

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?

356

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

[steveire]

Update

[steveire]

clang/docs/LibASTMatchersReference.html

303

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

[aaron.ballman]

clang/docs/LibASTMatchersReference.html

85–88

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?

303

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

[steveire]

clang/docs/LibASTMatchersReference.html

85–88

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

[steveire]

Update

[aaron.ballman]

LGTM, thank you for the new documentation!

clang/docs/LibASTMatchersReference.html

 of node that is expected. Every match expression starts with a node matcher,
 which can then be further refined with a narrowing or traversal matcher. All
 traversal matchers take node matchers as their arguments.</p>
 
 <p>For convenience, all node matchers take an arbitrary number of arguments
 and implicitly act as allOf matchers.</p>
 
 <p>Node matchers are the only matchers that support the bind("id") call to
 bind the matched node to the given string, to be later retrieved from the
 match callback.</p>
 
 <p>It is important to remember that the arguments to node matchers are
 predicates on the same node, just with additional information about the type.
 This is often useful to make matcher expression more readable by inlining bind
 calls into redundant node matchers inside another node matcher:
 <pre>
 // This binds the CXXRecordDecl to "id", as the decl() matcher will stay on
 // the same node.
 recordDecl(decl().bind("id"), hasName("::MyClass"))
 </pre>
 </p>
 
+<!-- ======================================================================= -->
+<h2 id="traverse-mode">Traverse Mode</h2>
+<!-- ======================================================================= -->
+
+<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
+that the user must write AST Matchers to explicitly traverse or ignore nodes
+which are not visible.</p>
+
+<p>In addition, because template instantations are matched in the default mode,
+transformations can be accidentally made to template declarations. Finally,
+because implicit nodes are matched by default, transformations can be made on
+entirely incorrect places in the code.</p>
+
+<p>For these reasons, it is possible to ignore AST nodes which are not spelled in
+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
+do not need to be encountered and matcher expressions adjusted for these
+cases.</p>
+
+<p>In clang-query, the mode can be changed with
+<pre>
+set traversal IgnoreUnlessSpelledInSource
+</pre>
+</p>
+This affects both matchers and AST dump output in results.
+
+<p>When using the C++ API such as in clang-tidy checks, the traverse() matcher
+is used to set the mode:
+<pre>
+Finder->addMatcher(traverse(TK_IgnoreUnlessSpelledInSource,
+  returnStmt(hasReturnArgument(integerLiteral(equals(0))))
+  ), this);
+</pre>
+</p>
+<p>The following table compares the AsIs mode with the IgnoreUnlessSpelledInSource
+mode:</p>
+
+
+<style>
+.traverse_compare, .traverse_compare td, .traverse_compare th {
+  border: 1px solid black;
+  border-collapse: collapse;
+}
+</style>
+<table class="traverse_compare">
+<tr>
+<th></th><th>AsIs</th><th>IgnoreUnlessSpelledInSource</th>
+</tr>
+<tr>
+  <td>AST dump of func1:
+<pre>
+struct B
+{
+  B(int);
+};
+
+B func1() {
+  return 42;
+}
+</pre>
+
+  </td>
+  <td>
+C++98 dialect:
+<pre>
+FunctionDecl
+`-CompoundStmt
+  `-ReturnStmt
+    `-ExprWithCleanups
+      `-CXXConstructExpr
+        `-MaterializeTemporaryExpr
+          `-ImplicitCastExpr
+            `-ImplicitCastExpr
+              `-CXXConstructExpr
+                `-IntegerLiteral 'int' 42
+</pre>
+C++11, C++14 dialect:
+<pre>
+FunctionDecl
+`-CompoundStmt
+  `-ReturnStmt
+    `-ExprWithCleanups
+      `-CXXConstructExpr
+        `-MaterializeTemporaryExpr
+          `-ImplicitCastExpr
+            `-CXXConstructExpr
+              `-IntegerLiteral 'int' 42
+</pre>
+C++17, C++20 dialect:
+<pre>
+FunctionDecl
+`-CompoundStmt
+  `-ReturnStmt
+    `-ImplicitCastExpr
+      `-CXXConstructExpr
+        `-IntegerLiteral 'int' 42
+</pre>
+</td>
+  <td>
+All dialects:
+    <pre>
+FunctionDecl
+`-CompoundStmt
+  `-ReturnStmt
+    `-IntegerLiteral 'int' 42
+</pre></td>
+</tr>
+
+<tr>
+<td>Matcher for returned '42':
+<pre>
+struct B
+{
+  B(int);
+};
+
+B func1() {
+  return 42;
+}
+</pre>
+
+  </td>
+  <td>
+All dialects:
+<pre>
+returnStmt(hasReturnValue(
+    ignoringImplicit(
+        ignoringElidableConstructorCall(
+            ignoringImplicit(
+                cxxConstructExpr(hasArgument(0,
+                    ignoringImplicit(
+                        integerLiteral().bind("returnVal")
+                        )
+                    ))
+                )
+            )
+        )
+    ))
+</pre></td>
+  <td>
+All dialects:
+<pre>
+returnStmt(hasReturnValue(
+    integerLiteral().bind("returnVal")
+))
+</pre></td>
+</tr>
+<tr>
+<td>Match result for
+<pre>implicitCastExpr()</pre>
+given:
+<pre>
+struct B
+{
+  B(int);
+};
+
+B func1() {
+  return 42;
+}
+</pre>
+
+</td>
+<td>
+Match found.</td>
+  <td>
+No match.</td>
+</tr>
+<tr>
+  <td>Match result for:
+<pre>
+cxxConstructorDecl(
+  isCopyConstructor()
+  ).bind("prepend_explicit")
+</pre>
+given:
+<pre>
+struct Other {
+};
+struct Copyable {
+  Other m_o;
+  Copyable();
+};
+</pre>
+</td>
+<td>
+Match found! Insertion produces incorrect output:
+<pre>
+struct Other {
+};
+struct explicit Copyable {
+  Other m_o;
+  Copyable();
+};
+</pre>
+</td>
+<td>
+No match found. Incorrect replacement not possible.
+</td>
+</tr>
+<tr>
+  <td>Replacement of `begin()` with `cbegin()`:
+<pre>
+cxxMemberCallExpr(
+  on(ConstContainerExpr),
+  callee(cxxMethodDecl(hasName("begin")))
+  ).bind("replace_with_cbegin")
+</pre>
+given:
+<pre>
+void foo()
+{
+  const Container c;
+  c.begin();
+
+  for (auto i : c)
+  {
+
+  }
+}
+</pre>
+</td>
+<td>
+2 matches found! Replacement produces incorrect output:
+<pre>
+void foo()
+{
+  const Container c;
+  c.cbegin();
+
+  for (auto i :.cbegin() c)
+  {
+
+  }
+}
+</pre>
+</td>
+<td>
+1 match found! Replacement produces correct output:
+<pre>
+void foo()
+{
+  const Container c;
+  c.cbegin();
+
+  for (auto i : c)
+  {
+
+  }
+}
+</pre>
+</td>
+</tr>
+<tr>
+  <td>Replacement of int member with safe_int:
+<pre>
+fieldDecl(
+  hasType(asString("int"))
+  ).bind("use_safe_int")
+</pre>
+given:
+<pre>
+struct S {
+  int m_i;
+};
+
+template&lt;typename T&gt;
+struct TemplStruct {
+  TemplStruct() {}
+  ~TemplStruct() {}
+
+private:
+  T m_t;
+};
+
+void instantiate()
+{
+  TemplStruct&lt;int&gt; ti;
+}
+</pre>
+</td>
+<td>
+2 matches found! Replacement produces incorrect output:
+<pre>
+struct S {
+  safe_int m_i;
+};
+
+template&lt;typename T&gt;
+struct TemplStruct {
+  TemplStruct() {}
+  ~TemplStruct() {}
+
+private:
+  safe_int m_t;
+};
+
+void instantiate()
+{
+  TemplStruct&lt;int&gt; ti;
+}
+</pre>
+</td>
+<td>
+1 match found! Replacement produces correct output:
+<pre>
+struct S {
+  safe_int m_i;
+};
+
+template&lt;typename T&gt;
+struct TemplStruct {
+  TemplStruct() {}
+  ~TemplStruct() {}
+
+private:
+  T m_t;
+};
+
+void instantiate()
+{
+  TemplStruct&lt;int&gt; ti;
+}
+</pre>
+</td>
+</tr>
+<tr>
+  <td>Add prefix to member initializer
+<pre>
+cxxCtorInitializer(
+  forField(fieldDecl())
+  ).bind("add_prefix")
+</pre>
+given:
+<pre>
+struct Simple {
+};
+
+struct Record {
+  Record() : i(42) {}
+private:
+  int i;
+  Simple s;
+};
+</pre>
+</td>
+<td>
+2 matches found! Replacement produces incorrect output:
+<pre>
+struct Simple {
+};
+
+struct Record {
+    m_Record() : m_i(42) {}
+private:
+  int i;
+  Simple s;
+};
+</pre>
+</td>
+<td>
+1 match found! Replacement produces correct output:
+<pre>
+struct Simple {
+};
+
+struct Record {
+    Record() : m_i(42) {}
+private:
+  int i;
+  Simple s;
+};
+</pre>
+</td>
+</tr>
+<tr>
+  <td>Ignored default arguments
+<pre>
+callExpr(
+  callee(functionDecl(
+    hasName("hasDefaultArg")
+    )),
+  argumentCountIs(1)
+  ).bind("add_prefix")
+</pre>
+given:
+<pre>
+void hasDefaultArg(int i, int j = 0)
+{
+}
+void callDefaultArg()
+{
+  hasDefaultArg(42);
+}
+</pre>
+</td>
+<td>
+No match!
+</td>
+<td>
+1 match found!
+</td>
+</tr>
+<tr>
+  <td>Lambda fields
+<pre>
+fieldDecl(
+  hasType(asString("int"))
+  ).bind("make_safe")
+</pre>
+given:
+<pre>
+struct S
+{
+    int m_i;
+};
+
+void func() {
+  int a = 0;
+  int c = 0;
+
+  auto l = [a, b = c](int d) { int e = d; };
+  l(43);
+}
+</pre>
+</td>
+<td>
+2 matches found! Replacement produces incorrect output:
+<pre>
+struct S
+{
+    safe_int m_i;
+};
+
+void func() {
+  int a = 0;
+  int c = 0;
+
+  auto l = [safe_a, safe_b = c](int d) { int e = d; };
+  l(43);
+}</pre>
+</td>
+<td>
+1 match found! Replacement produces correct output:
+<pre>
+struct S
+{
+    safe_int m_i;
+};
+
+void func() {
+  int a = 0;
+  int c = 0;
+
+  auto l = [a, b = c](int d) { int e = d; };
+  l(43);
+}</pre>
+</td>
+
+</tr>
+</table>
+
 <table>
 <tr style="text-align:left"><th>Return type</th><th>Name</th><th>Parameters</th></tr>
 <!-- START_DECL_MATCHERS -->
 
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXCtorInitializer.html">CXXCtorInitializer</a>&gt;</td><td class="name" onclick="toggle('cxxCtorInitializer0')"><a name="cxxCtorInitializer0Anchor">cxxCtorInitializer</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1CXXCtorInitializer.html">CXXCtorInitializer</a>&gt;...</td></tr>
 <tr><td colspan="4" class="doc" id="cxxCtorInitializer0"><pre>Matches constructor initializers.
 
 Examples matches i(42).