[PATCH] D65944: [clang] Update `ignoringElidableConstructorCall` matcher to ignore `ExprWithCleanups`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: gribozavr.
Herald added a project: clang.

The `ExprWithCleanups` node is added to the AST along with the elidable
CXXConstructExpr.  If it is the outermost node of the node being matched, ignore
it as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65944

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h


Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===================================================================
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6533,14 +6533,15 @@
 }
 
 /// Matches expressions that match InnerMatcher that are possibly wrapped in an
-/// elidable constructor.
+/// elidable constructor and other corresponding bookkeeping nodes.
 ///
-/// In C++17 copy elidable constructors are no longer being
-/// generated in the AST as it is not permitted by the standard. They are
-/// however part of the AST in C++14 and earlier. 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. This matcher can be used to
-/// skip those elidable constructors.
+/// In C++17 copy elidable constructors are no longer being generated in the AST
+/// as it is not permitted by the standard. They are, however, part of the AST
+/// in C++14 and earlier. So, a matcher must elide these differences to work in
+/// all language modes. This matcher supports such elision by skipping elidable
+/// constructor-call AST nodes, `ExprWithCleanups` nodes wrapping elidable
+/// constructor-calls and various implicit nodes inside the constructor calls,
+/// all of which will not appear in the C++17 AST.
 ///
 /// Given
 ///
@@ -6552,13 +6553,20 @@
 /// }
 /// \endcode
 ///
-/// ``varDecl(hasInitializer(any(
-///       ignoringElidableConstructorCall(callExpr()),
-///       exprWithCleanups(ignoringElidableConstructorCall(callExpr()))))``
-/// matches ``H D = G()``
+/// ``varDecl(hasInitializer(ignoringElidableConstructorCall(callExpr())))``
+/// matches ``H D = G()`` in C++11 through C++17 (and beyond).
 AST_MATCHER_P(Expr, ignoringElidableConstructorCall,
               ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
-  if (const auto *CtorExpr = dyn_cast<CXXConstructExpr>(&Node)) {
+  // E tracks the node that we are examining.
+  const Expr *E = &Node;
+  // If present, remove an outer `ExprWithCleanups` corresponding to the
+  // underlying `CXXConstructExpr`. This check won't cover all cases of added
+  // `ExprWithCleanups` corresponding to `CXXConstructExpr` nodes (because the
+  // EWC is placed on the outermost node of the expression, which this may not
+  // be), but, it still improves the coverage of this matcher.
+  if (const auto *CleanupsExpr = dyn_cast<ExprWithCleanups>(&Node))
+    E = CleanupsExpr->getSubExpr();
+  if (const auto *CtorExpr = dyn_cast<CXXConstructExpr>(E)) {
     if (CtorExpr->isElidable()) {
       if (const auto *MaterializeTemp =
               dyn_cast<MaterializeTemporaryExpr>(CtorExpr->getArg(0))) {
Index: clang/docs/LibASTMatchersReference.html
===================================================================
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -5805,14 +5805,15 @@
 
 <tr><td>Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>></td><td class="name" onclick="toggle('ignoringElidableConstructorCall0')"><a name="ignoringElidableConstructorCall0Anchor">ignoringElidableConstructorCall</a></td><td>ast_matchers::Matcher<<a href="https://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>> InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="ignoringElidableConstructorCall0"><pre>Matches expressions that match InnerMatcher that are possibly wrapped in an
-elidable constructor.
+elidable constructor and other corresponding bookkeeping nodes.
 
-In C++17 copy elidable constructors are no longer being
-generated in the AST as it is not permitted by the standard. They are
-however part of the AST in C++14 and earlier. 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. This matcher can be used to
-skip those elidable constructors.
+In C++17 copy elidable constructors are no longer being generated in the AST
+as it is not permitted by the standard. They are, however, part of the AST
+in C++14 and earlier. So, a matcher must elide these differences to work in
+all language modes. This matcher supports such elision by skipping elidable
+constructor-call AST nodes, `ExprWithCleanups` nodes wrapping elidable
+constructor-calls and various implicit nodes inside the constructor calls,
+all of which will not appear in the C++17 AST.
 
 Given
 
@@ -5822,10 +5823,8 @@
   H D = G();
 }
 
-``varDecl(hasInitializer(any(
-      ignoringElidableConstructorCall(callExpr()),
-      exprWithCleanups(ignoringElidableConstructorCall(callExpr()))))``
-matches ``H D = G()``
+``varDecl(hasInitializer(ignoringElidableConstructorCall(callExpr())))``
+matches ``H D = G()`` in C++11 through C++17 (and beyond).
 </pre></td></tr>
 
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65944.214123.patch
Type: text/x-patch
Size: 4882 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190808/4e7a7146/attachment-0001.bin>