[SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`

[eduucaldas]

Use IgnoreExpr.h infrastructure.

[eduucaldas]

nits

[eduucaldas]

clang/lib/Tooling/Syntax/BuildTree.cpp

47–57

Should this go into IgnoreExpr as well?

If yes, should we unify this with the lambda inside IgnoreUnlessSpelledInSource, thus removing the lambda and using this free function instead?

[gribozavr2]

clang/lib/Tooling/Syntax/BuildTree.cpp

47–57

That sounds like a good idea to me.

clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

1749

Could you also add a test that invokes implicit conversions?

struct X {
  X(int);
};
void TakeX(const X&);

void test() {
  [[TakeX(1)]];
}

[eduucaldas]

Add further tests and extract IgnoreImplicitConstructorSingleStep

[eduucaldas]

Add more tests, not extract IgnoreImplicitConstructorSingleStep

[eduucaldas]

.

[eduucaldas]

Add comments

[eduucaldas]

clang/lib/Tooling/Syntax/BuildTree.cpp

47–49

Please give feedback on this comments and should I comment the rest of the function?

[gribozavr2]

clang/lib/Tooling/Syntax/BuildTree.cpp

47

47–49

Seems straightforward to me.

clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

3863

This is not a conversion, this is an explicit constructor call (CXXTemporaryObjectExpr) -- so please rename the test. Same for other tests below.

[eduucaldas]

answer comments

clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

3863

I am trying to follow the syntax to name things. And in the grammar this is under "Explicit type conversion (functional notation)", which englobes as well things of the type float(3).

[eduucaldas]

• Rename tests
• Add FIXMEs for init-declarators

clang/lib/Tooling/Syntax/BuildTree.cpp

 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cstddef>
 #include <map>
 
 using namespace clang;
 
+static Expr *ignoreExprNodesImpl(Expr *E) { return E; }
+template <typename FnTy, typename... FnTys>
+static Expr *ignoreExprNodesImpl(Expr *E, FnTy &&Fn, FnTys &&...Fns) {
+  return ignoreExprNodesImpl(Fn(E), std::forward<FnTys>(Fns)...);
+}
+
+template <typename... FnTys>
+static Expr *ignoreUntilFixedPoint(Expr *E, FnTys &&...Fns) {
+  Expr *LastE = nullptr;
+  while (E != LastE) {
+    LastE = E;
+    E = ignoreExprNodesImpl(E, std::forward<FnTys>(Fns)...);
+    assert(E->getSourceRange() == LastE->getSourceRange());
+  }
+  return E;
+}
+
+static Expr *ignoreImplicitCXXConstructExpr(Expr *E) {
+  if (auto *C = dyn_cast<CXXConstructExpr>(E)) {
+    auto NumArgs = C->getNumArgs();
+    if (NumArgs == 1 || (NumArgs > 1 && isa<CXXDefaultArgExpr>(C->getArg(1)))) {
+      auto *A = C->getArg(0);
+      if (A->getSourceRange() == E->getSourceRange())
+        return A;
+    }
+  }
+  return E;
+}
+
+static Expr *ignoreImplicitCast(Expr *E) {
+  if (auto *ICE = dyn_cast<ImplicitCastExpr>(E)) {
+
+    auto *UnderlyingE = ICE->getSubExpr();
+    assert(UnderlyingE->getSourceRange() == E->getSourceRange());
+    return UnderlyingE;
+  }
+  return E;
+}
+
+static Expr *ignoreFull(Expr *E) {
+  if (auto *FE = dyn_cast<FullExpr>(E)) {
+    auto *UnderlyingE = FE->getSubExpr();
+    assert(UnderlyingE->getSourceRange() == E->getSourceRange());
+    return UnderlyingE;
+  }
+  return E;
+}
+
+static Expr *ignoreMaterializeTemporary(Expr *E) {
+  if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E)) {
+    auto *UnderlyingE = MTE->getSubExpr();
+    assert(UnderlyingE->getSourceRange() == E->getSourceRange());
+    return UnderlyingE;
+  }
+  return E;
+}
+
+static Expr *ignoreCXXBindTemporary(Expr *E) {
+  if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E)) {
+    auto *UnderlyingE = BTE->getSubExpr();
+    assert(UnderlyingE->getSourceRange() == E->getSourceRange());
+    return UnderlyingE;
+  }
+  return E;
+}
+
+static Expr *ignoreImplicit(Expr *E) {
+  return ignoreUntilFixedPoint(E, ignoreCXXBindTemporary, ignoreImplicitCast,
+                               ignoreMaterializeTemporary, ignoreFull,
+                               ignoreImplicitCXXConstructExpr);
+}
+
 LLVM_ATTRIBUTE_UNUSED
-static bool isImplicitExpr(Expr *E) { return E->IgnoreImplicit() != E; }
+static bool isImplicitExpr(Expr *E) { return ignoreImplicit(E) != E; }
 
 namespace {
 /// Get start location of the Declarator from the TypeLoc.
 /// E.g.:
 ///   loc of `(` in `int (a)`
 ///   loc of `*` in `int *(a)`
 ///   loc of the first `(` in `int (*a)(int)`
 ///   loc of the `*` in `int *(a)(int)`
public:
   }
 
   bool TraverseStmt(Stmt *S) {
     if (auto *DS = dyn_cast_or_null<DeclStmt>(S)) {
       // We want to consume the semicolon, make sure SimpleDeclaration does not.
       for (auto *D : DS->decls())
         Builder.noticeDeclWithoutSemicolon(D);
     } else if (auto *E = dyn_cast_or_null<Expr>(S)) {
-      return RecursiveASTVisitor::TraverseStmt(E->IgnoreImplicit());
+      return RecursiveASTVisitor::TraverseStmt(ignoreImplicit(E));
     }
     return RecursiveASTVisitor::TraverseStmt(S);
   }
 
   // Some expressions are not yet handled by syntax trees.
   bool WalkUpFromExpr(Expr *E) {
     assert(!isImplicitExpr(E) && "should be handled by TraverseStmt");
     Builder.foldNode(Builder.getExprRange(E),
void syntax::TreeBuilder::markStmtChild(Stmt *Child, NodeRole Role) {
   }
   assert(ChildNode != nullptr);
   setRole(ChildNode, Role);
 }
 
 void syntax::TreeBuilder::markExprChild(Expr *Child, NodeRole Role) {
   if (!Child)
     return;
-  Child = Child->IgnoreImplicit();
+  Child = ignoreImplicit(Child);
 
   syntax::Tree *ChildNode = Mapping.find(Child);
   assert(ChildNode != nullptr);
   setRole(ChildNode, Role);
 }
 
 const syntax::Token *syntax::TreeBuilder::findToken(SourceLocation L) const {
   if (L.isInvalid())

clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

TEST_P(SyntaxTreeTest, OverloadedOperator_Plus) {
   if (!GetParam().isCXX()) {
     return;
   }
   EXPECT_TRUE(treeDumpEqualOnAnnotations(
       R"cpp(
 struct X {
   friend X operator+(X, const X&);
 };
-// FIXME: Remove additional `UnknownExpression` wrapping `x`. For that, ignore
-// implicit copy constructor called on `x`. This should've been ignored already,
-// as we `IgnoreImplicit` when traversing an `Stmt`.
 void test(X x, X y) {
   [[x + y]];
 }
 )cpp",
       {R"txt(
 BinaryOperatorExpression Expression
-|-UnknownExpression LeftHandSide
-| `-IdExpression
-|   `-UnqualifiedId UnqualifiedId
-|     `-'x'
+|-IdExpression LeftHandSide
+| `-UnqualifiedId UnqualifiedId
+|   `-'x'
 |-'+' OperatorToken
 `-IdExpression RightHandSide
   `-UnqualifiedId UnqualifiedId
     `-'y'
 )txt"}));
 }
 
 TEST_P(SyntaxTreeTest, OverloadedOperator_Less) {
`-SimpleDeclaration
   | |-'='
   | `-UnknownExpression
   |   `-UnknownExpression
   |     |-'{'
   |     |-IntegerLiteralExpression
   |     | `-'1' LiteralToken
   |     |-','
   |     |-IntegerLiteralExpression
   |     | `-'2' LiteralToken
   |     |-','
   |     |-IntegerLiteralExpression
   |     | `-'3' LiteralToken
   |     `-'}'
   `-';'
 )txt"));
 }