Added AST matcher for ignoring elidable constructors

[jvikstrom]

Summary

Added AST matcher for ignoring elidable move constructors

[gribozavr]

clang/include/clang/ASTMatchers/ASTMatchers.h

6455

80 columns

6455

It would help if you described why a user would want to skip the elidable constructor using this matcher rather than hardcoding the presence of such an AST node.

6457

I'd suggest to rephrase the example using the "Given:" format (see other comments around), and wrap the code to 80 columns.

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

575

Can you run these tests in both C++14 and C++17 modes?

628

Need a space before the open brace, and indentation in the function body (in every test).

631

Please clang-format.

[jvikstrom]

• Made ignoreElidable also ignore materializeTemporaryExpr and reformatted code

[gribozavr]

clang/include/clang/ASTMatchers/ASTMatchers.h

6455

"Matches expressions that match InnerMatcher that are possibly wrapped in an elidable constructor."

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

6458

part of the AST in C++14 and earlier.

6460

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.

6467

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.

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

628

Not done? I was talking about the code in the code snippet.

[hokein]

clang/include/clang/ASTMatchers/ASTMatchers.h

6477

Is the matcher only strict to move constructor? looks like the matcher implementation is for general constructors, just ignoringElidableConstructorCall? we may have elidable copy constructor.

6479

nit: LLVM uses CamelCase style.

6486

I think this return is unnecessary, since we will fall through at line 6489.

[jvikstrom]

• Updated example and fixed edge case in ignoringElidableConstructorCall

[jvikstrom]

• Using CamelCase and also renamed ignoringElidableMoveConstructorCall to ignoringElidableConstructorCall

[jvikstrom]

• Added match conditionally overload to control in what language standard a match should run in

[jvikstrom]

• Fixed wrong formatting in test code

[gribozavr]

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

600

Please inline the variables that are used once.

clang/unittests/ASTMatchers/ASTMatchersTest.h

61

Please drop the explicit values.

64

2a

68

"2a" (we don't know the year yet)

144

80 columns.

154

No need for semicolon.

157

I'd say it is too clever, and suggest to write an explicit switch.

[jvikstrom]

• Using switch for choosing language standard

[gribozavr]

clang/unittests/ASTMatchers/ASTMatchersTest.h

68

Cxx2aOrLater?

(no need to uppercase things)

158

The "orlater" variants should not appear here, please use "default: llvm_unreachable()".

[jvikstrom]

• Added default to switch for language modes

[jvikstrom]

• Updated outdated docs

[jvikstrom]

• Removed dependency on unordered map

[hokein]

looks most good to me, a few nits.

clang/include/clang/ASTMatchers/ASTMatchers.h

6488

This return statement is not needed.

clang/unittests/ASTMatchers/ASTMatchersTest.h

16

looks like the include is not used?

152

nit: add a llvm_unreachable for default.

176

nit: no {} needed for a single-statement body.

[jvikstrom]

• Removed unnecessary return

[hokein]

thanks, looks good to me. I'll commit for you. @gribozavr do you want to take another look on the patch?

clang/unittests/ASTMatchers/ASTMatchersTest.h

152

this is not done?

[jvikstrom]

clang/unittests/ASTMatchers/ASTMatchersTest.h

152

But the switch covers all enum values so that would give warnings

clang/docs/LibASTMatchersReference.html

Example matches x (matcher = expr(hasType(cxxRecordDecl(hasName("X")))))
             and friend class X (matcher = friendDecl(hasType("X"))
  class X {};
  void y(X &x) { x; X z; }
  typedef int U;
  class Y { friend class X; };
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt;</td><td class="name" onclick="toggle('ignoringElidableConstructorCall0')"><a name="ignoringElidableConstructorCall0Anchor">ignoringElidableConstructorCall</a></td><td>ast_matchers::Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt; 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.
+
+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.
+
+Given
+
+struct H {};
+H G();
+void f() {
+  H D = G();
+}
+
+``varDecl(hasInitializer(any(
+      ignoringElidableConstructorCall(callExpr()),
+      exprWithCleanups(ignoringElidableConstructorCall(callExpr()))))``
+matches ``H D = G()``
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt;</td><td class="name" onclick="toggle('ignoringImpCasts0')"><a name="ignoringImpCasts0Anchor">ignoringImpCasts</a></td><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1Expr.html">Expr</a>&gt; InnerMatcher</td></tr>
 <tr><td colspan="4" class="doc" id="ignoringImpCasts0"><pre>Matches expressions that match InnerMatcher after any implicit casts
 are stripped off.
 
 Parentheses and explicit casts are not discarded.
 Given
   int arr[5];
   int a = 0;

clang/include/clang/ASTMatchers/ASTMatchers.h

 /// auto Y() -> int {}
 /// \endcode
 AST_MATCHER(FunctionDecl, hasTrailingReturn) {
   if (const auto *F = Node.getType()->getAs<FunctionProtoType>())
     return F->hasTrailingReturn();
   return false;
 }
 
+/// Matches expressions that match InnerMatcher that are possibly wrapped in an
+/// elidable constructor.
+///
+/// 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.
+///
+/// Given
+///
+/// \code
+/// struct H {};
+/// H G();
+/// void f() {
+///   H D = G();
+/// }
+/// \endcode
+///
+/// ``varDecl(hasInitializer(any(
+///       ignoringElidableConstructorCall(callExpr()),
+///       exprWithCleanups(ignoringElidableConstructorCall(callExpr()))))``
+/// matches ``H D = G()``
+AST_MATCHER_P(Expr, ignoringElidableConstructorCall,
+              ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+  if (const auto *CtorExpr = dyn_cast<CXXConstructExpr>(&Node)) {
+    if (CtorExpr->isElidable()) {
+      if (const auto *MaterializeTemp =
+              dyn_cast<MaterializeTemporaryExpr>(CtorExpr->getArg(0))) {
+        return InnerMatcher.matches(*MaterializeTemp->GetTemporaryExpr(),
+                                    Finder, Builder);
+      }
+    }
+  }
+  return InnerMatcher.matches(Node, Finder, Builder);
+}
+
 //----------------------------------------------------------------------------//
 // OpenMP handling.
 //----------------------------------------------------------------------------//
 
 /// Matches any ``#pragma omp`` executable directive.
 ///
 /// Given
 ///

clang/lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(hasTypeLoc);
   REGISTER_MATCHER(hasUnaryOperand);
   REGISTER_MATCHER(hasUnarySelector);
   REGISTER_MATCHER(hasUnderlyingDecl);
   REGISTER_MATCHER(hasUnderlyingType);
   REGISTER_MATCHER(hasUnqualifiedDesugaredType);
   REGISTER_MATCHER(hasValueType);
   REGISTER_MATCHER(ifStmt);
+  REGISTER_MATCHER(ignoringElidableConstructorCall);
   REGISTER_MATCHER(ignoringImpCasts);
   REGISTER_MATCHER(ignoringImplicit);
   REGISTER_MATCHER(ignoringParenCasts);
   REGISTER_MATCHER(ignoringParenImpCasts);
   REGISTER_MATCHER(imaginaryLiteral);
   REGISTER_MATCHER(implicitCastExpr);
   REGISTER_MATCHER(implicitValueInitExpr);
   REGISTER_MATCHER(incompleteArrayType);

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

TEST(Matcher, BindMatchedNodes) {
   StatementMatcher MethodX =
     callExpr(callee(cxxMethodDecl(hasName("x")))).bind("x");
 
   EXPECT_TRUE(matchAndVerifyResultTrue("class A { void x() { x(); } };",
                                        MethodX,
                                        llvm::make_unique<VerifyIdIsBoundTo<CXXMemberCallExpr>>("x")));
 }
 
+TEST(Matcher, IgnoresElidableConstructors) {
+  EXPECT_TRUE(
+      matches("struct H {};"
+              "template<typename T> H B(T A);"
+              "void f() {"
+              "  H D1;"
+              "  D1 = B(B(1));"
+              "}",
+              cxxOperatorCallExpr(hasArgument(
+                  1, callExpr(hasArgument(
+                         0, ignoringElidableConstructorCall(callExpr()))))),
+              LanguageMode::Cxx11OrLater));
+  EXPECT_TRUE(
+      matches("struct H {};"
+              "template<typename T> H B(T A);"
+              "void f() {"
+              "  H D1;"
+              "  D1 = B(1);"
+              "}",
+              cxxOperatorCallExpr(hasArgument(
+                  1, callExpr(hasArgument(0, ignoringElidableConstructorCall(
+                                                 integerLiteral()))))),
+              LanguageMode::Cxx11OrLater));
+  EXPECT_TRUE(matches(
+      "struct H {};"
+      "H G();"
+      "void f() {"
+      "  H D = G();"
+      "}",
+      varDecl(hasInitializer(anyOf(
+          ignoringElidableConstructorCall(callExpr()),
+          exprWithCleanups(has(ignoringElidableConstructorCall(callExpr())))))),
+      LanguageMode::Cxx11OrLater));
+}
+
+TEST(Matcher, IgnoresElidableInReturn) {
+  auto matcher = expr(ignoringElidableConstructorCall(declRefExpr()));
+  EXPECT_TRUE(matches("struct H {};"
+                      "H f() {"
+                      "  H g;"
+                      "  return g;"
+                      "}",
+                      matcher, LanguageMode::Cxx11OrLater));
+  EXPECT_TRUE(notMatches("struct H {};"
+                         "H f() {"
+                         "  return H();"
+                         "}",
+                         matcher, LanguageMode::Cxx11OrLater));
+}
+
+TEST(Matcher, IgnoreElidableConstructorDoesNotMatchConstructors) {
+  EXPECT_TRUE(matches("struct H {};"
+                      "void f() {"
+                      "  H D;"
+                      "}",
+                      varDecl(hasInitializer(
+                          ignoringElidableConstructorCall(cxxConstructExpr()))),
+                      LanguageMode::Cxx11OrLater));
+};
+
+TEST(Matcher, IgnoresElidableDoesNotPreventMatches) {
+  EXPECT_TRUE(matches("void f() {"
+                      "  int D = 10;"
+                      "}",
+                      expr(ignoringElidableConstructorCall(integerLiteral())),
+                      LanguageMode::Cxx11OrLater));
+}
+
 TEST(Matcher, BindTheSameNameInAlternatives) {
   StatementMatcher matcher = anyOf(
     binaryOperator(hasOperatorName("+"),
                    hasLHS(expr().bind("x")),
                    hasRHS(integerLiteral(equals(0)))),
     binaryOperator(hasOperatorName("+"),
                    hasLHS(integerLiteral(equals(0))),
                    hasRHS(expr().bind("x"))));
TEST(IsNoThrow, MatchesNoThrowFunctionDeclarations) {
   EXPECT_TRUE(matches("void f() noexcept;", functionProtoType(isNoThrow())));
 }
 
 TEST(isConstexpr, MatchesConstexprDeclarations) {
   EXPECT_TRUE(matches("constexpr int foo = 42;",
                       varDecl(hasName("foo"), isConstexpr())));
   EXPECT_TRUE(matches("constexpr int bar();",
                       functionDecl(hasName("bar"), isConstexpr())));
-  EXPECT_TRUE(matchesConditionally("void baz() { if constexpr(1 > 0) {} }",
-                                   ifStmt(isConstexpr()), true, "-std=c++17"));
-  EXPECT_TRUE(matchesConditionally("void baz() { if (1 > 0) {} }",
-                                   ifStmt(isConstexpr()), false, "-std=c++17"));
+  EXPECT_TRUE(matches("void baz() { if constexpr(1 > 0) {} }",
+                      ifStmt(isConstexpr()), LanguageMode::Cxx17OrLater));
+  EXPECT_TRUE(notMatches("void baz() { if (1 > 0) {} }", ifStmt(isConstexpr()),
+                         LanguageMode::Cxx17OrLater));
 }
 
 TEST(TemplateArgumentCountIs, Matches) {
   EXPECT_TRUE(
     matches("template<typename T> struct C {}; C<int> c;",
             classTemplateSpecializationDecl(templateArgumentCountIs(1))));
   EXPECT_TRUE(
     notMatches("template<typename T> struct C {}; C<int> c;",

clang/unittests/ASTMatchers/ASTMatchersTest.h

 //===- unittest/Tooling/ASTMatchersTest.h - Matcher tests helpers ------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_UNITTESTS_ASTMATCHERS_ASTMATCHERSTEST_H
 #define LLVM_CLANG_UNITTESTS_ASTMATCHERS_ASTMATCHERSTEST_H
 
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace ast_matchers {
 
 using clang::tooling::buildASTFromCodeWithArgs;
 using clang::tooling::newFrontendActionFactory;
 using clang::tooling::runToolOnCodeWithArgs;
 using clang::tooling::FrontendActionFactory;
 using clang::tooling::FileContentMappings;
if (FindResultReviewer)
       FindResultReviewer->onEndOfTranslationUnit();
   }
 
 private:
   bool *const Verified;
   const std::unique_ptr<BoundNodesCallback> FindResultReviewer;
 };
 
+enum class LanguageMode {
+  Cxx11,
+  Cxx14,
+  Cxx17,
+  Cxx2a,
+  Cxx11OrLater,
+  Cxx14OrLater,
+  Cxx17OrLater,
+  Cxx2aOrLater
+};
+
 template <typename T>
 testing::AssertionResult matchesConditionally(
     const std::string &Code, const T &AMatcher, bool ExpectMatch,
     llvm::ArrayRef<llvm::StringRef> CompileArgs,
     const FileContentMappings &VirtualMappedFiles = FileContentMappings(),
     const std::string &Filename = "input.cc") {
   bool Found = false, DynamicFound = false;
   MatchFinder Finder;
testing::AssertionResult matchesConditionally(
     const FileContentMappings &VirtualMappedFiles = FileContentMappings(),
     const std::string &Filename = "input.cc") {
   return matchesConditionally(Code, AMatcher, ExpectMatch,
                               llvm::makeArrayRef(CompileArg),
                               VirtualMappedFiles, Filename);
 }
 
 template <typename T>
-testing::AssertionResult matches(const std::string &Code, const T &AMatcher) {
-  return matchesConditionally(Code, AMatcher, true, "-std=c++11");
+testing::AssertionResult
+matchesConditionally(const std::string &Code, const T &AMatcher,
+                     bool ExpectMatch, const LanguageMode &Mode) {
+  std::vector<LanguageMode> LangModes;
+  switch (Mode) {
+  case LanguageMode::Cxx11:
+  case LanguageMode::Cxx14:
+  case LanguageMode::Cxx17:
+  case LanguageMode::Cxx2a:
+    LangModes = {Mode};
+    break;
+  case LanguageMode::Cxx11OrLater:
+    LangModes = {LanguageMode::Cxx11, LanguageMode::Cxx14, LanguageMode::Cxx17,
+                 LanguageMode::Cxx2a};
+    break;
+  case LanguageMode::Cxx14OrLater:
+    LangModes = {LanguageMode::Cxx14, LanguageMode::Cxx17, LanguageMode::Cxx2a};
+    break;
+  case LanguageMode::Cxx17OrLater:
+    LangModes = {LanguageMode::Cxx17, LanguageMode::Cxx2a};
+    break;
+  case LanguageMode::Cxx2aOrLater:
+    LangModes = {LanguageMode::Cxx2a};
+  }
+
+  for (auto Mode : LangModes) {
+    std::string LangModeArg;
+    switch (Mode) {
+    case LanguageMode::Cxx11:
+      LangModeArg = "-std=c++11";
+      break;
+    case LanguageMode::Cxx14:
+      LangModeArg = "-std=c++14";
+      break;
+    case LanguageMode::Cxx17:
+      LangModeArg = "-std=c++17";
+      break;
+    case LanguageMode::Cxx2a:
+      LangModeArg = "-std=c++2a";
+      break;
+    default:
+      llvm_unreachable("Invalid language mode");
+    }
+
+    auto Result =
+        matchesConditionally(Code, AMatcher, ExpectMatch, LangModeArg);
+    if (!Result)
+      return Result;
+  }
+
+  return testing::AssertionSuccess();
+}
+
+template <typename T>
+testing::AssertionResult
+matches(const std::string &Code, const T &AMatcher,
+        const LanguageMode &Modes = LanguageMode::Cxx11) {
+  return matchesConditionally(Code, AMatcher, true, Modes);
 }
 
 template <typename T>
-testing::AssertionResult notMatches(const std::string &Code,
-                                    const T &AMatcher) {
-  return matchesConditionally(Code, AMatcher, false, "-std=c++11");
+testing::AssertionResult
+notMatches(const std::string &Code, const T &AMatcher,
+           const LanguageMode &Modes = LanguageMode::Cxx11) {
+  return matchesConditionally(Code, AMatcher, false, Modes);
 }
 
 template <typename T>
 testing::AssertionResult matchesObjC(const std::string &Code, const T &AMatcher,
                                      bool ExpectMatch = true) {
   return matchesConditionally(Code, AMatcher, ExpectMatch,
                               {"-fobjc-nonfragile-abi", "-Wno-objc-root-class",
                                "-fblocks", "-Wno-incomplete-implementation"},