kimgr created this revision.
Herald added a subscriber: kristof.beyls.
kimgr requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

CastExpr::getSubExprAsWritten and getConversionFunction used to have
disparate implementations to traverse the sub-expression chain and skip
so-called "implicit temporaries" (which are really implicit nodes added by
Sema to represent semantic details in the AST).

There's some friction in these algorithms that makes it hard to extend and
change them:

- skipImplicitTemporary is order-dependent; it can skip a CXXBindTemporaryExpr 
nested inside a MaterializeTemporaryExpr, but not vice versa
- skipImplicitTemporary only runs one pass, it does not traverse multiple 
nested sequences of MTE/CBTE/MTE/CBTE, for example

Both of these weaknesses are void at this point, because this kind of
out-of-order multi-level nesting does not exist in the current AST.

Adding a new implicit expression to skip exacerbates the problem, however,
since a node X might show up in any and all locations between the existing.

Thus;

- Harmonize the form of getSubExprAsWritten and getConversionFunction so they 
both use a for loop
- Use the IgnoreExprNodes machinery to skip multiple nodes
- Rename skipImplicitTemporary to ignoreImplicitSemaNodes to generalize
- Update ignoreImplicitSemaNodes so it only skips one level per call, to mirror 
existing Ignore functions and work better with IgnoreExprNodes

This is a functional change, but one without visible effect.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119476

Files:
  clang/lib/AST/Expr.cpp


Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1899,51 +1899,48 @@
 }
 
 namespace {
-  const Expr *skipImplicitTemporary(const Expr *E) {
-    // Skip through reference binding to temporary.
-    if (auto *Materialize = dyn_cast<MaterializeTemporaryExpr>(E))
-      E = Materialize->getSubExpr();
+// Skip over implicit nodes produced as part of semantic analysis.
+// Designed for use with IgnoreExpreNodes.
+Expr *ignoreImplicitSemaNodes(Expr *E) {
+  if (auto *Materialize = dyn_cast<MaterializeTemporaryExpr>(E))
+    return Materialize->getSubExpr();
 
-    // Skip any temporary bindings; they're implicit.
-    if (auto *Binder = dyn_cast<CXXBindTemporaryExpr>(E))
-      E = Binder->getSubExpr();
+  if (auto *Binder = dyn_cast<CXXBindTemporaryExpr>(E))
+    return Binder->getSubExpr();
 
-    return E;
-  }
+  return E;
 }
+} // namespace
 
 Expr *CastExpr::getSubExprAsWritten() {
   const Expr *SubExpr = nullptr;
-  const CastExpr *E = this;
-  do {
-    SubExpr = skipImplicitTemporary(E->getSubExpr());
+
+  for (const CastExpr *E = this; E; E = dyn_cast<ImplicitCastExpr>(SubExpr)) {
+    SubExpr = IgnoreExprNodes(E->getSubExpr(), ignoreImplicitSemaNodes);
 
     // Conversions by constructor and conversion functions have a
     // subexpression describing the call; strip it off.
-    if (E->getCastKind() == CK_ConstructorConversion)
-      SubExpr =
-        
skipImplicitTemporary(cast<CXXConstructExpr>(SubExpr->IgnoreImplicit())->getArg(0));
-    else if (E->getCastKind() == CK_UserDefinedConversion) {
+    if (E->getCastKind() == CK_ConstructorConversion) {
+      SubExpr = IgnoreExprNodes(
+          cast<CXXConstructExpr>(SubExpr->IgnoreImplicit())->getArg(0),
+          ignoreImplicitSemaNodes);
+    } else if (E->getCastKind() == CK_UserDefinedConversion) {
       SubExpr = SubExpr->IgnoreImplicit();
-      assert((isa<CXXMemberCallExpr>(SubExpr) ||
-              isa<BlockExpr>(SubExpr)) &&
+      assert((isa<CXXMemberCallExpr>(SubExpr) || isa<BlockExpr>(SubExpr)) &&
              "Unexpected SubExpr for CK_UserDefinedConversion.");
       if (auto *MCE = dyn_cast<CXXMemberCallExpr>(SubExpr))
         SubExpr = MCE->getImplicitObjectArgument();
     }
+  }
 
-    // If the subexpression we're left with is an implicit cast, look
-    // through that, too.
-  } while ((E = dyn_cast<ImplicitCastExpr>(SubExpr)));
-
-  return const_cast<Expr*>(SubExpr);
+  return const_cast<Expr *>(SubExpr);
 }
 
 NamedDecl *CastExpr::getConversionFunction() const {
   const Expr *SubExpr = nullptr;
 
   for (const CastExpr *E = this; E; E = dyn_cast<ImplicitCastExpr>(SubExpr)) {
-    SubExpr = skipImplicitTemporary(E->getSubExpr());
+    SubExpr = IgnoreExprNodes(E->getSubExpr(), ignoreImplicitSemaNodes);
 
     if (E->getCastKind() == CK_ConstructorConversion)
       return cast<CXXConstructExpr>(SubExpr)->getConstructor();


Index: clang/lib/AST/Expr.cpp
===================================================================
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -1899,51 +1899,48 @@
 }
 
 namespace {
-  const Expr *skipImplicitTemporary(const Expr *E) {
-    // Skip through reference binding to temporary.
-    if (auto *Materialize = dyn_cast<MaterializeTemporaryExpr>(E))
-      E = Materialize->getSubExpr();
+// Skip over implicit nodes produced as part of semantic analysis.
+// Designed for use with IgnoreExpreNodes.
+Expr *ignoreImplicitSemaNodes(Expr *E) {
+  if (auto *Materialize = dyn_cast<MaterializeTemporaryExpr>(E))
+    return Materialize->getSubExpr();
 
-    // Skip any temporary bindings; they're implicit.
-    if (auto *Binder = dyn_cast<CXXBindTemporaryExpr>(E))
-      E = Binder->getSubExpr();
+  if (auto *Binder = dyn_cast<CXXBindTemporaryExpr>(E))
+    return Binder->getSubExpr();
 
-    return E;
-  }
+  return E;
 }
+} // namespace
 
 Expr *CastExpr::getSubExprAsWritten() {
   const Expr *SubExpr = nullptr;
-  const CastExpr *E = this;
-  do {
-    SubExpr = skipImplicitTemporary(E->getSubExpr());
+
+  for (const CastExpr *E = this; E; E = dyn_cast<ImplicitCastExpr>(SubExpr)) {
+    SubExpr = IgnoreExprNodes(E->getSubExpr(), ignoreImplicitSemaNodes);
 
     // Conversions by constructor and conversion functions have a
     // subexpression describing the call; strip it off.
-    if (E->getCastKind() == CK_ConstructorConversion)
-      SubExpr =
-        skipImplicitTemporary(cast<CXXConstructExpr>(SubExpr->IgnoreImplicit())->getArg(0));
-    else if (E->getCastKind() == CK_UserDefinedConversion) {
+    if (E->getCastKind() == CK_ConstructorConversion) {
+      SubExpr = IgnoreExprNodes(
+          cast<CXXConstructExpr>(SubExpr->IgnoreImplicit())->getArg(0),
+          ignoreImplicitSemaNodes);
+    } else if (E->getCastKind() == CK_UserDefinedConversion) {
       SubExpr = SubExpr->IgnoreImplicit();
-      assert((isa<CXXMemberCallExpr>(SubExpr) ||
-              isa<BlockExpr>(SubExpr)) &&
+      assert((isa<CXXMemberCallExpr>(SubExpr) || isa<BlockExpr>(SubExpr)) &&
              "Unexpected SubExpr for CK_UserDefinedConversion.");
       if (auto *MCE = dyn_cast<CXXMemberCallExpr>(SubExpr))
         SubExpr = MCE->getImplicitObjectArgument();
     }
+  }
 
-    // If the subexpression we're left with is an implicit cast, look
-    // through that, too.
-  } while ((E = dyn_cast<ImplicitCastExpr>(SubExpr)));
-
-  return const_cast<Expr*>(SubExpr);
+  return const_cast<Expr *>(SubExpr);
 }
 
 NamedDecl *CastExpr::getConversionFunction() const {
   const Expr *SubExpr = nullptr;
 
   for (const CastExpr *E = this; E; E = dyn_cast<ImplicitCastExpr>(SubExpr)) {
-    SubExpr = skipImplicitTemporary(E->getSubExpr());
+    SubExpr = IgnoreExprNodes(E->getSubExpr(), ignoreImplicitSemaNodes);
 
     if (E->getCastKind() == CK_ConstructorConversion)
       return cast<CXXConstructExpr>(SubExpr)->getConstructor();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to