[PATCH] D63149: Added AST matcher for ignoring elidable constructors

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6455
 
+/// Matches expressions that match InnerMatcher after any elidable constructor
+/// are stripped off. In C++17 copy elidable constructors are no longer being
----------------
"Matches expressions that match InnerMatcher that are possibly wrapped in an elidable constructor."

Then a blank line, and the rest of the explanation.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6458
+/// generated in the AST as it is not permitted by the standard. They are
+/// however part of the C++14 AST. This means that there are cases where it is
+/// needed to use ``anyOf(cxxConstructorExpr(Inner), Inner)`` to match for both
----------------
part of the AST in C++14 and earlier.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6460
+/// needed to use ``anyOf(cxxConstructorExpr(Inner), Inner)`` to match for both
+/// the C++14 and C++17 AST. Instead of doing this, this matcher can be used as
+/// ``ignoringElidableMoveConstructorCall(Inner)``.
----------------
Therefore, to write a matcher that works in all language modes, the matcher has to skip elidable constructor AST nodes if they appear in the AST.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:6467
+/// struct H {};
+/// template<typename T> H B(T A);
+/// void f() {
----------------
Can this sample be simplified? For example, B does not need to be a template (I think), and f() can be simplified to not call B twice.


================
Comment at: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp:628
+  EXPECT_TRUE(matches(
+    "void f(){"
+    "int D = 10;"
----------------
gribozavr wrote:
> Need a space before the open brace, and indentation in the function body (in every test).
Not done? I was talking about the code in the code snippet.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63149/new/

https://reviews.llvm.org/D63149