Extract ExprTraversal class from Expr

[steveire]

Summary

The current implementations are concerned with descending through the
AST. An addition to the API will be concerned with ascending through
the AST in different traversal modes. Keep both in the same class to
ensure that they are kept in sync.

[steveire]

Format

[aaron.ballman]

This looks like it's an NFC patch where the only change is to hoist this functionality out into its own interface. Is that correct? If so, can you please be sure to add NFC to the commit log?

clang/include/clang/AST/ExprTraversal.h

10

Some comments explaining the purpose of the file would be appreciated.

22

Any particular reason why this is a struct rather than a namespace given that there's no state?

23

Should we be adding some documentation comments for these APIs?

[rsmith]

An addition to the API will be concerned with ascending through the AST in different traversal modes.

Ascending through the AST is not possibly by design. (For example, we share AST nodes between template instantiations, and statement nodes don't contain parent pointers.) Can you say more about what you're thinking of adding here?

clang/include/clang/AST/ExprTraversal.h

2

This needs an update.

clang/lib/AST/ExprTraversal.cpp

2

This header should briefly describe the purpose of this file. ("Simple expression traversal"?)

[steveire]

Update for review comments

[steveire]

Update

[steveire]

In D73028#1839383, @rsmith wrote:

An addition to the API will be concerned with ascending through the AST in different traversal modes.

Ascending through the AST is not possibly by design. (For example, we share AST nodes between template instantiations, and statement nodes don't contain parent pointers.) Can you say more about what you're thinking of adding here?

The follow-up is here: https://reviews.llvm.org/D73029 .

I have the need to change the ascending traversal implemented in ASTContext.cpp with a map (populated while descending) to make it skip nodes too during parent traversal.

I didn't want to have descending traversal in Expr.cpp and ascending traversal in ASTContext.cpp as they would be likely to go out of sync, so moving this implementation here allows extending the purpose in the follow-up.

[rsmith]

In D73028#1839494, @steveire wrote:

In D73028#1839383, @rsmith wrote:

The follow-up is here: https://reviews.llvm.org/D73029 .

I have the need to change the ascending traversal implemented in ASTContext.cpp with a map (populated while descending) to make it skip nodes too during parent traversal.

I didn't want to have descending traversal in Expr.cpp and ascending traversal in ASTContext.cpp as they would be likely to go out of sync, so moving this implementation here allows extending the purpose in the follow-up.

The ascending traversal should not be in the AST library at all. If tooling or static analysis wants this, that's fine, but navigating upwards is not something the AST supports, and we do not want any part of clang proper developing a reliance on having a parent map, so we're working on moving the parent map out of ASTContext (with a plan to eventually move it out of the AST library). See D71313 for related work in this area.

[aaron.ballman]

In D73028#1839572, @rsmith wrote:

In D73028#1839494, @steveire wrote:

In D73028#1839383, @rsmith wrote:

The follow-up is here: https://reviews.llvm.org/D73029 .

I have the need to change the ascending traversal implemented in ASTContext.cpp with a map (populated while descending) to make it skip nodes too during parent traversal.

I didn't want to have descending traversal in Expr.cpp and ascending traversal in ASTContext.cpp as they would be likely to go out of sync, so moving this implementation here allows extending the purpose in the follow-up.

The ascending traversal should not be in the AST library at all. If tooling or static analysis wants this, that's fine, but navigating upwards is not something the AST supports, and we do not want any part of clang proper developing a reliance on having a parent map, so we're working on moving the parent map out of ASTContext (with a plan to eventually move it out of the AST library). See D71313 for related work in this area.

Thank you for pointing this out, I was unaware of that direction.

[steveire]

In D73028#1839572, @rsmith wrote:

In D73028#1839494, @steveire wrote:

In D73028#1839383, @rsmith wrote:

The follow-up is here: https://reviews.llvm.org/D73029 .

I have the need to change the ascending traversal implemented in ASTContext.cpp with a map (populated while descending) to make it skip nodes too during parent traversal.

I didn't want to have descending traversal in Expr.cpp and ascending traversal in ASTContext.cpp as they would be likely to go out of sync, so moving this implementation here allows extending the purpose in the follow-up.

The ascending traversal should not be in the AST library at all. If tooling or static analysis wants this, that's fine, but navigating upwards is not something the AST supports, and we do not want any part of clang proper developing a reliance on having a parent map, so we're working on moving the parent map out of ASTContext (with a plan to eventually move it out of the AST library). See D71313 for related work in this area.

Thanks for the heads-up - I'm trying to reach the conclusion from this information.

Are you saying that

1. this patch shouldn't be committed, so down-traversal-skipping should remain in Expr.cpp
2. D73029 should be changed to implement the up-traversal-skipping directly in ParentMapContext.cpp

Is that the tight conclusion?

[steveire]

In D73028#1839644, @steveire wrote:

In D73028#1839572, @rsmith wrote:

In D73028#1839494, @steveire wrote:

In D73028#1839383, @rsmith wrote:

The follow-up is here: https://reviews.llvm.org/D73029 .

I have the need to change the ascending traversal implemented in ASTContext.cpp with a map (populated while descending) to make it skip nodes too during parent traversal.

I didn't want to have descending traversal in Expr.cpp and ascending traversal in ASTContext.cpp as they would be likely to go out of sync, so moving this implementation here allows extending the purpose in the follow-up.

The ascending traversal should not be in the AST library at all. If tooling or static analysis wants this, that's fine, but navigating upwards is not something the AST supports, and we do not want any part of clang proper developing a reliance on having a parent map, so we're working on moving the parent map out of ASTContext (with a plan to eventually move it out of the AST library). See D71313 for related work in this area.

Thanks for the heads-up - I'm trying to reach the conclusion from this information.

Are you saying that

1. this patch shouldn't be committed, so down-traversal-skipping should remain in Expr.cpp
2. D73029 should be changed to implement the up-traversal-skipping directly in ParentMapContext.cpp

Is that the right conclusion?

Implemented that in https://reviews.llvm.org/D73388

I think this and https://reviews.llvm.org/D73029 can be closed in favor of that one.

clang/include/clang/AST/Expr.h

public:
   /// Returns true if this expression is a bound member function.
   bool isBoundMemberFunction(ASTContext &Ctx) const;
 
   /// Given an expression of bound-member type, find the type
   /// of the member.  Returns null if this is an *overloaded* bound
   /// member expression.
   static QualType findBoundMemberType(const Expr *expr);
 
-  /// Skip past any invisble AST nodes which might surround this
-  /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes,
-  /// but also injected CXXMemberExpr and CXXConstructExpr which represent
-  /// implicit conversions.
+  /// Forwards to expr_traversal::DescendIgnoreUnlessSpelledInSource
   Expr *IgnoreUnlessSpelledInSource();
   const Expr *IgnoreUnlessSpelledInSource() const {
     return const_cast<Expr *>(this)->IgnoreUnlessSpelledInSource();
   }
 
-  /// Skip past any implicit casts which might surround this expression until
-  /// reaching a fixed point. Skips:
-  /// * ImplicitCastExpr
-  /// * FullExpr
+  /// Forwards to expr_traversal::DescendIgnoreImpCasts
   Expr *IgnoreImpCasts() LLVM_READONLY;
   const Expr *IgnoreImpCasts() const {
     return const_cast<Expr *>(this)->IgnoreImpCasts();
   }
 
-  /// Skip past any casts which might surround this expression until reaching
-  /// a fixed point. Skips:
-  /// * CastExpr
-  /// * FullExpr
-  /// * MaterializeTemporaryExpr
-  /// * SubstNonTypeTemplateParmExpr
+  /// Forwards to expr_traversal::DescendIgnoreCasts
   Expr *IgnoreCasts() LLVM_READONLY;
   const Expr *IgnoreCasts() const {
     return const_cast<Expr *>(this)->IgnoreCasts();
   }
 
-  /// Skip past any implicit AST nodes which might surround this expression
-  /// until reaching a fixed point. Skips:
-  /// * What IgnoreImpCasts() skips
-  /// * MaterializeTemporaryExpr
-  /// * CXXBindTemporaryExpr
+  /// Forwards to expr_traversal::DescendIgnoreImplicit
   Expr *IgnoreImplicit() LLVM_READONLY;
   const Expr *IgnoreImplicit() const {
     return const_cast<Expr *>(this)->IgnoreImplicit();
   }
 
-  /// Skip past any implicit AST nodes which might surround this expression
-  /// until reaching a fixed point. Same as IgnoreImplicit, except that it
-  /// also skips over implicit calls to constructors and conversion functions.
-  ///
-  /// FIXME: Should IgnoreImplicit do this?
+  /// Forwards to expr_traversal::DescendIgnoreImplicitAsWritten
   Expr *IgnoreImplicitAsWritten() LLVM_READONLY;
   const Expr *IgnoreImplicitAsWritten() const {
     return const_cast<Expr *>(this)->IgnoreImplicitAsWritten();
   }
 
-  /// Skip past any parentheses which might surround this expression until
-  /// reaching a fixed point. Skips:
-  /// * ParenExpr
-  /// * UnaryOperator if `UO_Extension`
-  /// * GenericSelectionExpr if `!isResultDependent()`
-  /// * ChooseExpr if `!isConditionDependent()`
-  /// * ConstantExpr
+  /// Forwards to expr_traversal::DescendIgnoreParens
   Expr *IgnoreParens() LLVM_READONLY;
   const Expr *IgnoreParens() const {
     return const_cast<Expr *>(this)->IgnoreParens();
   }
 
-  /// Skip past any parentheses and implicit casts which might surround this
-  /// expression until reaching a fixed point.
-  /// FIXME: IgnoreParenImpCasts really ought to be equivalent to
-  /// IgnoreParens() + IgnoreImpCasts() until reaching a fixed point. However
-  /// this is currently not the case. Instead IgnoreParenImpCasts() skips:
-  /// * What IgnoreParens() skips
-  /// * What IgnoreImpCasts() skips
-  /// * MaterializeTemporaryExpr
-  /// * SubstNonTypeTemplateParmExpr
+  /// Forwards to expr_traversal::DescendIgnoreParenImpCasts
   Expr *IgnoreParenImpCasts() LLVM_READONLY;
   const Expr *IgnoreParenImpCasts() const {
     return const_cast<Expr *>(this)->IgnoreParenImpCasts();
   }
 
-  /// Skip past any parentheses and casts which might surround this expression
-  /// until reaching a fixed point. Skips:
-  /// * What IgnoreParens() skips
-  /// * What IgnoreCasts() skips
+  /// Forwards to expr_traversal::DescendIgnoreParenCasts
   Expr *IgnoreParenCasts() LLVM_READONLY;
   const Expr *IgnoreParenCasts() const {
     return const_cast<Expr *>(this)->IgnoreParenCasts();
   }
 
-  /// Skip conversion operators. If this Expr is a call to a conversion
-  /// operator, return the argument.
+  /// Forwards to expr_traversal::DescendIgnoreConversionOperator
   Expr *IgnoreConversionOperator() LLVM_READONLY;
   const Expr *IgnoreConversionOperator() const {
     return const_cast<Expr *>(this)->IgnoreConversionOperator();
   }
 
-  /// Skip past any parentheses and lvalue casts which might surround this
-  /// expression until reaching a fixed point. Skips:
-  /// * What IgnoreParens() skips
-  /// * What IgnoreCasts() skips, except that only lvalue-to-rvalue
-  ///   casts are skipped
-  /// FIXME: This is intended purely as a temporary workaround for code
-  /// that hasn't yet been rewritten to do the right thing about those
-  /// casts, and may disappear along with the last internal use.
+  /// Forwards to expr_traversal::DescendIgnoreParenLValueCasts
   Expr *IgnoreParenLValueCasts() LLVM_READONLY;
   const Expr *IgnoreParenLValueCasts() const {
     return const_cast<Expr *>(this)->IgnoreParenLValueCasts();
   }
 
-  /// Skip past any parenthese and casts which do not change the value
-  /// (including ptr->int casts of the same size) until reaching a fixed point.
-  /// Skips:
-  /// * What IgnoreParens() skips
-  /// * CastExpr which do not change the value
-  /// * SubstNonTypeTemplateParmExpr
+  /// Forwards to expr_traversal::DescendIgnoreParenNoopCasts
   Expr *IgnoreParenNoopCasts(const ASTContext &Ctx) LLVM_READONLY;
   const Expr *IgnoreParenNoopCasts(const ASTContext &Ctx) const {
     return const_cast<Expr *>(this)->IgnoreParenNoopCasts(Ctx);
   }
 
-  /// Skip past any parentheses and derived-to-base casts until reaching a
-  /// fixed point. Skips:
-  /// * What IgnoreParens() skips
-  /// * CastExpr which represent a derived-to-base cast (CK_DerivedToBase,
-  ///   CK_UncheckedDerivedToBase and CK_NoOp)
+  /// Forwards to expr_traversal::DescendIgnoreParenBaseCasts
   Expr *ignoreParenBaseCasts() LLVM_READONLY;
   const Expr *ignoreParenBaseCasts() const {
     return const_cast<Expr *>(this)->ignoreParenBaseCasts();
   }
 
   /// Determine whether this expression is a default function argument.
   ///
   /// Default arguments are implicitly generated in the abstract syntax tree

clang/include/clang/AST/ExprTraversal.h

+//===--- ExprTraversal.h - Simple expression traversal --------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines the expr_traversal namespace. It contains functions
+//  used to skip nodes while traversing the AST.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_AST_EXPR_TRAVERSAL_H
+#define LLVM_CLANG_AST_EXPR_TRAVERSAL_H
+
+namespace clang {
+
+class ASTContext;
+class Expr;
+
+namespace expr_traversal {
+
+/// Skip past any implicit casts which might surround this expression until
+/// reaching a fixed point. Skips:
+/// * ImplicitCastExpr
+/// * FullExpr
+Expr *DescendIgnoreImpCasts(Expr *E);
+
+/// Skip past any casts which might surround this expression until reaching
+/// a fixed point. Skips:
+/// * CastExpr
+/// * FullExpr
+/// * MaterializeTemporaryExpr
+/// * SubstNonTypeTemplateParmExpr
+Expr *DescendIgnoreCasts(Expr *E);
+
+/// Skip past any implicit AST nodes which might surround this expression
+/// until reaching a fixed point. Skips:
+/// * What IgnoreImpCasts() skips
+/// * MaterializeTemporaryExpr
+/// * CXXBindTemporaryExpr
+Expr *DescendIgnoreImplicit(Expr *E);
+
+/// Skip past any implicit AST nodes which might surround this expression
+/// until reaching a fixed point. Same as IgnoreImplicit, except that it
+/// also skips over implicit calls to constructors and conversion functions.
+///
+/// FIXME: Should IgnoreImplicit do this?
+Expr *DescendIgnoreImplicitAsWritten(Expr *E);
+
+/// Skip past any parentheses which might surround this expression until
+/// reaching a fixed point. Skips:
+/// * ParenExpr
+/// * UnaryOperator if `UO_Extension`
+/// * GenericSelectionExpr if `!isResultDependent()`
+/// * ChooseExpr if `!isConditionDependent()`
+/// * ConstantExpr
+Expr *DescendIgnoreParens(Expr *E);
+
+/// Skip past any parentheses and implicit casts which might surround this
+/// expression until reaching a fixed point.
+/// FIXME: IgnoreParenImpCasts really ought to be equivalent to
+/// IgnoreParens() + IgnoreImpCasts() until reaching a fixed point. However
+/// this is currently not the case. Instead IgnoreParenImpCasts() skips:
+/// * What IgnoreParens() skips
+/// * What IgnoreImpCasts() skips
+/// * MaterializeTemporaryExpr
+/// * SubstNonTypeTemplateParmExpr
+Expr *DescendIgnoreParenImpCasts(Expr *E);
+
+/// Skip past any parentheses and casts which might surround this expression
+/// until reaching a fixed point. Skips:
+/// * What IgnoreParens() skips
+/// * What IgnoreCasts() skips
+Expr *DescendIgnoreParenCasts(Expr *E);
+
+/// Skip conversion operators. If this Expr is a call to a conversion
+/// operator, return the argument.
+Expr *DescendIgnoreConversionOperator(Expr *E);
+
+/// Skip past any parentheses and lvalue casts which might surround this
+/// expression until reaching a fixed point. Skips:
+/// * What IgnoreParens() skips
+/// * What IgnoreCasts() skips, except that only lvalue-to-rvalue
+///   casts are skipped
+/// FIXME: This is intended purely as a temporary workaround for code
+/// that hasn't yet been rewritten to do the right thing about those
+/// casts, and may disappear along with the last internal use.
+Expr *DescendIgnoreParenLValueCasts(Expr *E);
+
+/// Skip past any parentheses and derived-to-base casts until reaching a
+/// fixed point. Skips:
+/// * What IgnoreParens() skips
+/// * CastExpr which represent a derived-to-base cast (CK_DerivedToBase,
+///   CK_UncheckedDerivedToBase and CK_NoOp)
+Expr *DescendIgnoreParenBaseCasts(Expr *E);
+
+/// Skip past any parenthese and casts which do not change the value
+/// (including ptr->int casts of the same size) until reaching a fixed point.
+/// Skips:
+/// * What IgnoreParens() skips
+/// * CastExpr which do not change the value
+/// * SubstNonTypeTemplateParmExpr
+Expr *DescendIgnoreParenNoopCasts(const ASTContext &Ctx, Expr *E);
+
+/// Skip past any invisble AST nodes which might surround this
+/// statement, such as ExprWithCleanups or ImplicitCastExpr nodes,
+/// but also injected CXXMemberExpr and CXXConstructExpr which represent
+/// implicit conversions.
+Expr *DescendIgnoreUnlessSpelledInSource(Expr *E);
+}; // namespace expr_traversal
+
+} // namespace clang
+
+#endif

clang/lib/AST/CMakeLists.txt

add_clang_library(clangAST
   DeclPrinter.cpp
   DeclTemplate.cpp
   Expr.cpp
   ExprClassification.cpp
   ExprConcepts.cpp
   ExprConstant.cpp
   ExprCXX.cpp
   ExprObjC.cpp
+  ExprTraversal.cpp
   ExternalASTMerger.cpp
   ExternalASTSource.cpp
   FormatString.cpp
   InheritViz.cpp
   Interp/Block.cpp
   Interp/ByteCodeEmitter.cpp
   Interp/ByteCodeExprGen.cpp
   Interp/ByteCodeGenError.cpp

clang/lib/AST/Expr.cpp

 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExprTraversal.h"
 #include "clang/AST/Mangle.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Lexer.h"
if (const BinaryOperator *op = dyn_cast<BinaryOperator>(expr)) {
     assert(type->isFunctionType());
     return type;
   }
 
   assert(isa<UnresolvedMemberExpr>(expr) || isa<CXXPseudoDestructorExpr>(expr));
   return QualType();
 }
 
-static Expr *IgnoreImpCastsSingleStep(Expr *E) {
-  if (auto *ICE = dyn_cast<ImplicitCastExpr>(E))
-    return ICE->getSubExpr();
-
-  if (auto *FE = dyn_cast<FullExpr>(E))
-    return FE->getSubExpr();
-
-  return E;
-}
-
-static Expr *IgnoreImpCastsExtraSingleStep(Expr *E) {
-  // FIXME: Skip MaterializeTemporaryExpr and SubstNonTypeTemplateParmExpr in
-  // addition to what IgnoreImpCasts() skips to account for the current
-  // behaviour of IgnoreParenImpCasts().
-  Expr *SubE = IgnoreImpCastsSingleStep(E);
-  if (SubE != E)
-    return SubE;
-
-  if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E))
-    return MTE->getSubExpr();
-
-  if (auto *NTTP = dyn_cast<SubstNonTypeTemplateParmExpr>(E))
-    return NTTP->getReplacement();
-
-  return E;
-}
-
-static Expr *IgnoreCastsSingleStep(Expr *E) {
-  if (auto *CE = dyn_cast<CastExpr>(E))
-    return CE->getSubExpr();
-
-  if (auto *FE = dyn_cast<FullExpr>(E))
-    return FE->getSubExpr();
-
-  if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E))
-    return MTE->getSubExpr();
-
-  if (auto *NTTP = dyn_cast<SubstNonTypeTemplateParmExpr>(E))
-    return NTTP->getReplacement();
-
-  return E;
-}
-
-static Expr *IgnoreLValueCastsSingleStep(Expr *E) {
-  // Skip what IgnoreCastsSingleStep skips, except that only
-  // lvalue-to-rvalue casts are skipped.
-  if (auto *CE = dyn_cast<CastExpr>(E))
-    if (CE->getCastKind() != CK_LValueToRValue)
-      return E;
-
-  return IgnoreCastsSingleStep(E);
-}
-
-static Expr *IgnoreBaseCastsSingleStep(Expr *E) {
-  if (auto *CE = dyn_cast<CastExpr>(E))
-    if (CE->getCastKind() == CK_DerivedToBase ||
-        CE->getCastKind() == CK_UncheckedDerivedToBase ||
-        CE->getCastKind() == CK_NoOp)
-      return CE->getSubExpr();
-
-  return E;
-}
-
-static Expr *IgnoreImplicitSingleStep(Expr *E) {
-  Expr *SubE = IgnoreImpCastsSingleStep(E);
-  if (SubE != E)
-    return SubE;
-
-  if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E))
-    return MTE->getSubExpr();
-
-  if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
-    return BTE->getSubExpr();
-
-  return E;
-}
-
-static Expr *IgnoreImplicitAsWrittenSingleStep(Expr *E) {
-  if (auto *ICE = dyn_cast<ImplicitCastExpr>(E))
-    return ICE->getSubExprAsWritten();
-
-  return IgnoreImplicitSingleStep(E);
-}
-
-static Expr *IgnoreParensOnlySingleStep(Expr *E) {
-  if (auto *PE = dyn_cast<ParenExpr>(E))
-    return PE->getSubExpr();
-  return E;
-}
-
-static Expr *IgnoreParensSingleStep(Expr *E) {
-  if (auto *PE = dyn_cast<ParenExpr>(E))
-    return PE->getSubExpr();
-
-  if (auto *UO = dyn_cast<UnaryOperator>(E)) {
-    if (UO->getOpcode() == UO_Extension)
-      return UO->getSubExpr();
-  }
-
-  else if (auto *GSE = dyn_cast<GenericSelectionExpr>(E)) {
-    if (!GSE->isResultDependent())
-      return GSE->getResultExpr();
-  }
-
-  else if (auto *CE = dyn_cast<ChooseExpr>(E)) {
-    if (!CE->isConditionDependent())
-      return CE->getChosenSubExpr();
-  }
-
-  else if (auto *CE = dyn_cast<ConstantExpr>(E))
-    return CE->getSubExpr();
-
-  return E;
-}
-
-static Expr *IgnoreNoopCastsSingleStep(const ASTContext &Ctx, Expr *E) {
-  if (auto *CE = dyn_cast<CastExpr>(E)) {
-    // We ignore integer <-> casts that are of the same width, ptr<->ptr and
-    // ptr<->int casts of the same width. We also ignore all identity casts.
-    Expr *SubExpr = CE->getSubExpr();
-    bool IsIdentityCast =
-        Ctx.hasSameUnqualifiedType(E->getType(), SubExpr->getType());
-    bool IsSameWidthCast =
-        (E->getType()->isPointerType() || E->getType()->isIntegralType(Ctx)) &&
-        (SubExpr->getType()->isPointerType() ||
-         SubExpr->getType()->isIntegralType(Ctx)) &&
-        (Ctx.getTypeSize(E->getType()) == Ctx.getTypeSize(SubExpr->getType()));
-
-    if (IsIdentityCast || IsSameWidthCast)
-      return SubExpr;
-  }
-
-  else if (auto *NTTP = dyn_cast<SubstNonTypeTemplateParmExpr>(E))
-    return NTTP->getReplacement();
-
-  return E;
-}
-
-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)...);
-}
-
-/// Given an expression E and functions Fn_1,...,Fn_n : Expr * -> Expr *,
-/// Recursively apply each of the functions to E until reaching a fixed point.
-/// Note that a null E is valid; in this case nothing is done.
-template <typename... FnTys>
-static Expr *IgnoreExprNodes(Expr *E, FnTys &&... Fns) {
-  Expr *LastE = nullptr;
-  while (E != LastE) {
-    LastE = E;
-    E = IgnoreExprNodesImpl(E, std::forward<FnTys>(Fns)...);
-  }
-  return E;
-}
-
 Expr *Expr::IgnoreImpCasts() {
-  return IgnoreExprNodes(this, IgnoreImpCastsSingleStep);
+  return expr_traversal::DescendIgnoreImpCasts(this);
 }
 
-Expr *Expr::IgnoreCasts() {
-  return IgnoreExprNodes(this, IgnoreCastsSingleStep);
-}
+Expr *Expr::IgnoreCasts() { return expr_traversal::DescendIgnoreCasts(this); }
 
 Expr *Expr::IgnoreImplicit() {
-  return IgnoreExprNodes(this, IgnoreImplicitSingleStep);
+  return expr_traversal::DescendIgnoreImplicit(this);
 }
 
 Expr *Expr::IgnoreImplicitAsWritten() {
-  return IgnoreExprNodes(this, IgnoreImplicitAsWrittenSingleStep);
+  return expr_traversal::DescendIgnoreImplicitAsWritten(this);
 }
 
-Expr *Expr::IgnoreParens() {
-  return IgnoreExprNodes(this, IgnoreParensSingleStep);
-}
+Expr *Expr::IgnoreParens() { return expr_traversal::DescendIgnoreParens(this); }
 
 Expr *Expr::IgnoreParenImpCasts() {
-  return IgnoreExprNodes(this, IgnoreParensSingleStep,
-                         IgnoreImpCastsExtraSingleStep);
+  return expr_traversal::DescendIgnoreParenImpCasts(this);
 }
 
 Expr *Expr::IgnoreParenCasts() {
-  return IgnoreExprNodes(this, IgnoreParensSingleStep, IgnoreCastsSingleStep);
+  return expr_traversal::DescendIgnoreParenCasts(this);
 }
 
 Expr *Expr::IgnoreConversionOperator() {
-  if (auto *MCE = dyn_cast<CXXMemberCallExpr>(this)) {
-    if (MCE->getMethodDecl() && isa<CXXConversionDecl>(MCE->getMethodDecl()))
-      return MCE->getImplicitObjectArgument();
-  }
-  return this;
+  return expr_traversal::DescendIgnoreConversionOperator(this);
 }
 
 Expr *Expr::IgnoreParenLValueCasts() {
-  return IgnoreExprNodes(this, IgnoreParensSingleStep,
-                         IgnoreLValueCastsSingleStep);
+  return expr_traversal::DescendIgnoreParenLValueCasts(this);
 }
 
 Expr *Expr::ignoreParenBaseCasts() {
-  return IgnoreExprNodes(this, IgnoreParensSingleStep,
-                         IgnoreBaseCastsSingleStep);
+  return expr_traversal::DescendIgnoreParenBaseCasts(this);
 }
 
 Expr *Expr::IgnoreParenNoopCasts(const ASTContext &Ctx) {
-  return IgnoreExprNodes(this, IgnoreParensSingleStep, [&Ctx](Expr *E) {
-    return IgnoreNoopCastsSingleStep(Ctx, E);
-  });
+  return expr_traversal::DescendIgnoreParenNoopCasts(Ctx, this);
 }
 
 Expr *Expr::IgnoreUnlessSpelledInSource() {
-  Expr *E = this;
-
-  Expr *LastE = nullptr;
-  while (E != LastE) {
-    LastE = E;
-    E = IgnoreExprNodes(E, IgnoreImplicitSingleStep, IgnoreImpCastsSingleStep,
-                        IgnoreParensOnlySingleStep);
-
-    auto SR = E->getSourceRange();
-
-    if (auto *C = dyn_cast<CXXConstructExpr>(E)) {
-      if (C->getNumArgs() == 1) {
-        Expr *A = C->getArg(0);
-        if (A->getSourceRange() == SR || !isa<CXXTemporaryObjectExpr>(C))
-          E = A;
-      }
-    }
-
-    if (auto *C = dyn_cast<CXXMemberCallExpr>(E)) {
-      Expr *ExprNode = C->getImplicitObjectArgument()->IgnoreParenImpCasts();
-      if (ExprNode->getSourceRange() == SR)
-        E = ExprNode;
-    }
-  }
-
-  return E;
+  return expr_traversal::DescendIgnoreUnlessSpelledInSource(this);
 }
 
 bool Expr::isDefaultArgument() const {
   const Expr *E = this;
   if (const MaterializeTemporaryExpr *M = dyn_cast<MaterializeTemporaryExpr>(E))
     E = M->getSubExpr();
 
   while (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E))

clang/lib/AST/ExprTraversal.cpp

+//===--- ExprTraversal.cpp - Simple expression traversal ------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements Expr traversal.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/ExprTraversal.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
+
+using namespace clang;
+
+static Expr *IgnoreImpCastsSingleStep(Expr *E) {
+  if (auto *ICE = dyn_cast<ImplicitCastExpr>(E))
+    return ICE->getSubExpr();
+
+  if (auto *FE = dyn_cast<FullExpr>(E))
+    return FE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreImpCastsExtraSingleStep(Expr *E) {
+  // FIXME: Skip MaterializeTemporaryExpr and SubstNonTypeTemplateParmExpr in
+  // addition to what IgnoreImpCasts() skips to account for the current
+  // behaviour of IgnoreParenImpCasts().
+  Expr *SubE = IgnoreImpCastsSingleStep(E);
+  if (SubE != E)
+    return SubE;
+
+  if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E))
+    return MTE->getSubExpr();
+
+  if (auto *NTTP = dyn_cast<SubstNonTypeTemplateParmExpr>(E))
+    return NTTP->getReplacement();
+
+  return E;
+}
+
+static Expr *IgnoreCastsSingleStep(Expr *E) {
+  if (auto *CE = dyn_cast<CastExpr>(E))
+    return CE->getSubExpr();
+
+  if (auto *FE = dyn_cast<FullExpr>(E))
+    return FE->getSubExpr();
+
+  if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E))
+    return MTE->getSubExpr();
+
+  if (auto *NTTP = dyn_cast<SubstNonTypeTemplateParmExpr>(E))
+    return NTTP->getReplacement();
+
+  return E;
+}
+
+static Expr *IgnoreLValueCastsSingleStep(Expr *E) {
+  // Skip what IgnoreCastsSingleStep skips, except that only
+  // lvalue-to-rvalue casts are skipped.
+  if (auto *CE = dyn_cast<CastExpr>(E))
+    if (CE->getCastKind() != CK_LValueToRValue)
+      return E;
+
+  return IgnoreCastsSingleStep(E);
+}
+
+static Expr *IgnoreBaseCastsSingleStep(Expr *E) {
+  if (auto *CE = dyn_cast<CastExpr>(E))
+    if (CE->getCastKind() == CK_DerivedToBase ||
+        CE->getCastKind() == CK_UncheckedDerivedToBase ||
+        CE->getCastKind() == CK_NoOp)
+      return CE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreImplicitSingleStep(Expr *E) {
+  Expr *SubE = IgnoreImpCastsSingleStep(E);
+  if (SubE != E)
+    return SubE;
+
+  if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E))
+    return MTE->getSubExpr();
+
+  if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
+    return BTE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreImplicitAsWrittenSingleStep(Expr *E) {
+  if (auto *ICE = dyn_cast<ImplicitCastExpr>(E))
+    return ICE->getSubExprAsWritten();
+
+  return IgnoreImplicitSingleStep(E);
+}
+
+static Expr *IgnoreParensOnlySingleStep(Expr *E) {
+  if (auto *PE = dyn_cast<ParenExpr>(E))
+    return PE->getSubExpr();
+  return E;
+}
+
+static Expr *IgnoreParensSingleStep(Expr *E) {
+  if (auto *PE = dyn_cast<ParenExpr>(E))
+    return PE->getSubExpr();
+
+  if (auto *UO = dyn_cast<UnaryOperator>(E)) {
+    if (UO->getOpcode() == UO_Extension)
+      return UO->getSubExpr();
+  }
+
+  else if (auto *GSE = dyn_cast<GenericSelectionExpr>(E)) {
+    if (!GSE->isResultDependent())
+      return GSE->getResultExpr();
+  }
+
+  else if (auto *CE = dyn_cast<ChooseExpr>(E)) {
+    if (!CE->isConditionDependent())
+      return CE->getChosenSubExpr();
+  }
+
+  else if (auto *CE = dyn_cast<ConstantExpr>(E))
+    return CE->getSubExpr();
+
+  return E;
+}
+
+static Expr *IgnoreNoopCastsSingleStep(const ASTContext &Ctx, Expr *E) {
+  if (auto *CE = dyn_cast<CastExpr>(E)) {
+    // We ignore integer <-> casts that are of the same width, ptr<->ptr and
+    // ptr<->int casts of the same width. We also ignore all identity casts.
+    Expr *SubExpr = CE->getSubExpr();
+    bool IsIdentityCast =
+        Ctx.hasSameUnqualifiedType(E->getType(), SubExpr->getType());
+    bool IsSameWidthCast =
+        (E->getType()->isPointerType() || E->getType()->isIntegralType(Ctx)) &&
+        (SubExpr->getType()->isPointerType() ||
+         SubExpr->getType()->isIntegralType(Ctx)) &&
+        (Ctx.getTypeSize(E->getType()) == Ctx.getTypeSize(SubExpr->getType()));
+
+    if (IsIdentityCast || IsSameWidthCast)
+      return SubExpr;
+  }
+
+  else if (auto *NTTP = dyn_cast<SubstNonTypeTemplateParmExpr>(E))
+    return NTTP->getReplacement();
+
+  return E;
+}
+
+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)...);
+}
+
+/// Given an expression E and functions Fn_1,...,Fn_n : Expr * -> Expr *,
+/// Recursively apply each of the functions to E until reaching a fixed point.
+/// Note that a null E is valid; in this case nothing is done.
+template <typename... FnTys>
+static Expr *IgnoreExprNodes(Expr *E, FnTys &&... Fns) {
+  Expr *LastE = nullptr;
+  while (E != LastE) {
+    LastE = E;
+    E = IgnoreExprNodesImpl(E, std::forward<FnTys>(Fns)...);
+  }
+  return E;
+}
+
+Expr *expr_traversal::DescendIgnoreImpCasts(Expr *E) {
+  return IgnoreExprNodes(E, IgnoreImpCastsSingleStep);
+}
+
+Expr *expr_traversal::DescendIgnoreCasts(Expr *E) {
+  return IgnoreExprNodes(E, IgnoreCastsSingleStep);
+}
+
+Expr *expr_traversal::DescendIgnoreImplicit(Expr *E) {
+  return IgnoreExprNodes(E, IgnoreImplicitSingleStep);
+}
+
+Expr *expr_traversal::DescendIgnoreImplicitAsWritten(Expr *E) {
+  return IgnoreExprNodes(E, IgnoreImplicitAsWrittenSingleStep);
+}
+
+Expr *expr_traversal::DescendIgnoreParens(Expr *E) {
+  return IgnoreExprNodes(E, IgnoreParensSingleStep);
+}
+
+Expr *expr_traversal::DescendIgnoreParenImpCasts(Expr *E) {
+  return IgnoreExprNodes(E, IgnoreParensSingleStep,
+                         IgnoreImpCastsExtraSingleStep);
+}
+
+Expr *expr_traversal::DescendIgnoreParenCasts(Expr *E) {
+  return IgnoreExprNodes(E, IgnoreParensSingleStep, IgnoreCastsSingleStep);
+}
+
+Expr *expr_traversal::DescendIgnoreConversionOperator(Expr *E) {
+  if (auto *MCE = dyn_cast<CXXMemberCallExpr>(E)) {
+    if (MCE->getMethodDecl() && isa<CXXConversionDecl>(MCE->getMethodDecl()))
+      return MCE->getImplicitObjectArgument();
+  }
+  return E;
+}
+
+Expr *expr_traversal::DescendIgnoreParenLValueCasts(Expr *E) {
+  return IgnoreExprNodes(E, IgnoreParensSingleStep,
+                         IgnoreLValueCastsSingleStep);
+}
+
+Expr *expr_traversal::DescendIgnoreParenBaseCasts(Expr *E) {
+  return IgnoreExprNodes(E, IgnoreParensSingleStep, IgnoreBaseCastsSingleStep);
+}
+
+Expr *expr_traversal::DescendIgnoreParenNoopCasts(const ASTContext &Ctx,
+                                                  Expr *Input) {
+  return IgnoreExprNodes(Input, IgnoreParensSingleStep, [&Ctx](Expr *E) {
+    return IgnoreNoopCastsSingleStep(Ctx, E);
+  });
+}
+
+Expr *expr_traversal::DescendIgnoreUnlessSpelledInSource(Expr *E) {
+  Expr *LastE = nullptr;
+  while (E != LastE) {
+    LastE = E;
+    E = IgnoreExprNodes(E, IgnoreImplicitSingleStep, IgnoreImpCastsSingleStep,
+                        IgnoreParensOnlySingleStep);
+
+    auto SR = E->getSourceRange();
+
+    if (auto *C = dyn_cast<CXXConstructExpr>(E)) {
+      if (C->getNumArgs() == 1) {
+        Expr *A = C->getArg(0);
+        if (A->getSourceRange() == SR || !isa<CXXTemporaryObjectExpr>(C))
+          E = A;
+      }
+    }
+
+    if (auto *C = dyn_cast<CXXMemberCallExpr>(E)) {
+      Expr *ExprNode = C->getImplicitObjectArgument()->IgnoreParenImpCasts();
+      if (ExprNode->getSourceRange() == SR)
+        E = ExprNode;
+    }
+  }
+
+  return E;
+}