riccibruno created this revision.
riccibruno added a reviewer: aaron.ballman.
riccibruno added a project: clang.
Herald added a subscriber: cfe-commits.
The description of what the various `Expr::Ignore*` do has drifted from the
actual implementation.
Inspection reveals that `IgnoreParenImpCasts()` is not equivalent to doing
`IgnoreParens()` + `IgnoreImpCasts()` until reaching a fixed point, but
`IgnoreParenCasts()` is equivalent to doing `IgnoreParens() + IgnoreCasts()`
until reaching a fixed point.
There is also a fair amount of duplication in the various `Expr::Ignore*`
functions which increase the chance of further future inconsistencies.
In preparation for the next patch which will factor out the implementation of
the various `Expr::Ignore*`, do the following cleanups:
Remove `Stmt::IgnoreImplicit`, in favor of `Expr::IgnoreImplicit`.
`IgnoreImplicit` is the only function among all of the `Expr::Ignore*` which is
available in `Stmt`. There are only a few users of `Stmt::IgnoreImplicit`. They
can just use instead `Expr::IgnoreImplicit` like they have to do for the other
`Ignore*`.
Move `Expr::IgnoreImpCasts()` from `Expr.h` to `Expr.cpp`. This made no
difference in the run-time with my usual benchmark (-fsyntax-only on all of
Boost).
While we are at it, make `IgnoreParenNoopCasts` take a const reference to the
`ASTContext` for const correctness.
Update the comments to match what the `Expr::Ignore*` are actually doing. I am
not sure that listing exactly what each `Expr::Ignore*` do is optimal, but it
certainly looks better than the current state which is in my opinion between
misleading and just plain wrong.
Repository:
rC Clang
https://reviews.llvm.org/D57266
Files:
include/clang/AST/Expr.h
include/clang/AST/Stmt.h
lib/ARCMigrate/TransRetainReleaseDealloc.cpp
lib/ARCMigrate/TransformActions.cpp
lib/ARCMigrate/Transforms.cpp
lib/AST/Expr.cpp
lib/AST/Stmt.cpp
lib/Analysis/ReachableCode.cpp
lib/Tooling/ASTDiff/ASTDiff.cpp
Index: lib/Tooling/ASTDiff/ASTDiff.cpp
===================================================================
--- lib/Tooling/ASTDiff/ASTDiff.cpp
+++ lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -237,8 +237,8 @@
return true;
}
bool TraverseStmt(Stmt *S) {
- if (S)
- S = S->IgnoreImplicit();
+ if (auto *E = dyn_cast_or_null<Expr>(S))
+ S = E->IgnoreImplicit();
if (isNodeExcluded(Tree.AST.getSourceManager(), S))
return true;
auto SavedState = PreTraverse(S);
Index: lib/Analysis/ReachableCode.cpp
===================================================================
--- lib/Analysis/ReachableCode.cpp
+++ lib/Analysis/ReachableCode.cpp
@@ -192,10 +192,12 @@
if (!S)
return false;
- S = S->IgnoreImplicit();
+ if (const auto *Ex = dyn_cast<Expr>(S))
+ S = Ex->IgnoreImplicit();
- if (const Expr *Ex = dyn_cast<Expr>(S))
+ if (const auto *Ex = dyn_cast<Expr>(S)) {
S = Ex->IgnoreCasts();
+ }
// Special case looking for the sigil '()' around an integer literal.
if (const ParenExpr *PE = dyn_cast<ParenExpr>(S))
Index: lib/AST/Stmt.cpp
===================================================================
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -117,30 +117,6 @@
StatisticsEnabled = true;
}
-Stmt *Stmt::IgnoreImplicit() {
- Stmt *s = this;
-
- Stmt *lasts = nullptr;
-
- while (s != lasts) {
- lasts = s;
-
- if (auto *fe = dyn_cast<FullExpr>(s))
- s = fe->getSubExpr();
-
- if (auto *mte = dyn_cast<MaterializeTemporaryExpr>(s))
- s = mte->GetTemporaryExpr();
-
- if (auto *bte = dyn_cast<CXXBindTemporaryExpr>(s))
- s = bte->getSubExpr();
-
- if (auto *ice = dyn_cast<ImplicitCastExpr>(s))
- s = ice->getSubExpr();
- }
-
- return s;
-}
-
/// Skip no-op (attributed, compound) container stmts and skip captured
/// stmt at the top, if \a IgnoreCaptured is true.
Stmt *Stmt::IgnoreContainers(bool IgnoreCaptured) {
Index: lib/AST/Expr.cpp
===================================================================
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -2556,6 +2556,39 @@
return QualType();
}
+Expr *Expr::IgnoreImpCasts() {
+ Expr *E = this;
+ while (true)
+ if (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E))
+ E = ICE->getSubExpr();
+ else if (FullExpr *FE = dyn_cast<FullExpr>(E))
+ E = FE->getSubExpr();
+ else
+ break;
+ return E;
+}
+
+Expr *Expr::IgnoreImplicit() {
+ Expr *E = this;
+ Expr *LastE = nullptr;
+ while (E != LastE) {
+ LastE = E;
+
+ if (auto *ICE = dyn_cast<ImplicitCastExpr>(E))
+ E = ICE->getSubExpr();
+
+ if (auto *FE = dyn_cast<FullExpr>(E))
+ E = FE->getSubExpr();
+
+ if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(E))
+ E = MTE->GetTemporaryExpr();
+
+ if (auto *BTE = dyn_cast<CXXBindTemporaryExpr>(E))
+ E = BTE->getSubExpr();
+ }
+ return E;
+}
+
Expr* Expr::IgnoreParens() {
Expr* E = this;
while (true) {
@@ -2722,7 +2755,7 @@
/// IgnoreParenNoopCasts - Ignore parentheses and casts that do not change the
/// value (including ptr->int casts of the same size). Strip off any
/// ParenExpr or CastExprs, returning their operand.
-Expr *Expr::IgnoreParenNoopCasts(ASTContext &Ctx) {
+Expr *Expr::IgnoreParenNoopCasts(const ASTContext &Ctx) {
Expr *E = this;
while (true) {
E = E->IgnoreParens();
Index: lib/ARCMigrate/Transforms.cpp
===================================================================
--- lib/ARCMigrate/Transforms.cpp
+++ lib/ARCMigrate/Transforms.cpp
@@ -288,9 +288,11 @@
while (LabelStmt *Label = dyn_cast<LabelStmt>(S))
S = Label->getSubStmt();
- S = S->IgnoreImplicit();
- if (Expr *E = dyn_cast<Expr>(S))
+ if (auto *E = dyn_cast<Expr>(S))
+ S = E->IgnoreImplicit();
+ if (Expr *E = dyn_cast<Expr>(S)) {
Removables.insert(E);
+ }
}
};
Index: lib/ARCMigrate/TransformActions.cpp
===================================================================
--- lib/ARCMigrate/TransformActions.cpp
+++ lib/ARCMigrate/TransformActions.cpp
@@ -313,7 +313,9 @@
assert(IsInTransaction && "Actions only allowed during a transaction");
ActionData data;
data.Kind = Act_RemoveStmt;
- data.S = S->IgnoreImplicit(); // important for uniquing
+ if (auto *E = dyn_cast<Expr>(S))
+ S = E->IgnoreImplicit(); // important for uniquing
+ data.S = S;
CachedActions.push_back(data);
}
Index: lib/ARCMigrate/TransRetainReleaseDealloc.cpp
===================================================================
--- lib/ARCMigrate/TransRetainReleaseDealloc.cpp
+++ lib/ARCMigrate/TransRetainReleaseDealloc.cpp
@@ -269,8 +269,8 @@
if (prevChildS != childE) {
prevStmt = *prevChildS;
- if (prevStmt)
- prevStmt = prevStmt->IgnoreImplicit();
+ if (auto *E = dyn_cast_or_null<Expr>(prevStmt))
+ prevStmt = E->IgnoreImplicit();
}
if (currChildS == childE)
@@ -280,8 +280,8 @@
return std::make_pair(prevStmt, nextStmt);
nextStmt = *currChildS;
- if (nextStmt)
- nextStmt = nextStmt->IgnoreImplicit();
+ if (auto *E = dyn_cast_or_null<Expr>(nextStmt))
+ nextStmt = E->IgnoreImplicit();
return std::make_pair(prevStmt, nextStmt);
}
Index: include/clang/AST/Stmt.h
===================================================================
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -1068,13 +1068,6 @@
/// works on systems with GraphViz (Mac OS X) or dot+gv installed.
void viewAST() const;
- /// Skip past any implicit AST nodes which might surround this
- /// statement, such as ExprWithCleanups or ImplicitCastExpr nodes.
- Stmt *IgnoreImplicit();
- const Stmt *IgnoreImplicit() const {
- return const_cast<Stmt *>(this)->IgnoreImplicit();
- }
-
/// Skip no-op (attributed, compound) container stmts and skip captured
/// stmt at the top, if \a IgnoreCaptured is true.
Stmt *IgnoreContainers(bool IgnoreCaptured = false);
Index: include/clang/AST/Expr.h
===================================================================
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -744,67 +744,110 @@
/// member expression.
static QualType findBoundMemberType(const Expr *expr);
- /// IgnoreImpCasts - Skip past any implicit casts which might
- /// surround this expression. Only skips ImplicitCastExprs.
+ /// Skip past any implicit casts which might surround this expression until
+ /// reaching a fixed point. Skips:
+ /// * ImplicitCastExpr.
+ /// * FullExpr.
Expr *IgnoreImpCasts() LLVM_READONLY;
-
- /// IgnoreImplicit - Skip past any implicit AST nodes which might
- /// surround this expression.
- Expr *IgnoreImplicit() LLVM_READONLY {
- return cast<Expr>(Stmt::IgnoreImplicit());
- }
-
- const Expr *IgnoreImplicit() const LLVM_READONLY {
- return const_cast<Expr*>(this)->IgnoreImplicit();
+ const Expr *IgnoreImpCasts() const {
+ return const_cast<Expr *>(this)->IgnoreImpCasts();
}
- /// IgnoreParens - Ignore parentheses. If this Expr is a ParenExpr, return
- /// its subexpression. If that subexpression is also a ParenExpr,
- /// then this method recursively returns its subexpression, and so forth.
- /// Otherwise, the method returns the current Expr.
- Expr *IgnoreParens() LLVM_READONLY;
-
- /// IgnoreParenCasts - Ignore parentheses and casts. Strip off any ParenExpr
- /// or CastExprs, returning their operand.
- Expr *IgnoreParenCasts() LLVM_READONLY;
-
- /// Ignore casts. Strip off any CastExprs, returning their operand.
+ /// Skip past any casts which might surround this expression until reaching
+ /// a fixed point. Skips:
+ /// * CastExpr.
+ /// * FullExpr.
+ /// * MaterializeTemporaryExpr.
+ /// * SubstNonTypeTemplateParmExpr.
Expr *IgnoreCasts() LLVM_READONLY;
-
- /// IgnoreParenImpCasts - Ignore parentheses and implicit casts. Strip off
- /// any ParenExpr or ImplicitCastExprs, returning their operand.
+ 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.
+ Expr *IgnoreImplicit() LLVM_READONLY;
+ const Expr *IgnoreImplicit() const {
+ return const_cast<Expr *>(this)->IgnoreImplicit();
+ }
+
+ /// 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 *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.
+ /// * ImplicitCastExpr.
+ /// * MaterializeTemporaryExpr.
+ /// * SubstNonTypeTemplateParmExpr.
Expr *IgnoreParenImpCasts() LLVM_READONLY;
-
- /// IgnoreConversionOperator - Ignore conversion operator. If this Expr is a
- /// call to a conversion operator, return the argument.
- Expr *IgnoreConversionOperator() LLVM_READONLY;
-
- const Expr *IgnoreConversionOperator() const LLVM_READONLY {
- return const_cast<Expr*>(this)->IgnoreConversionOperator();
+ const Expr *IgnoreParenImpCasts() const {
+ return const_cast<Expr *>(this)->IgnoreParenImpCasts();
}
- const Expr *IgnoreParenImpCasts() const LLVM_READONLY {
- 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.
+ Expr *IgnoreParenCasts() LLVM_READONLY;
+ const Expr *IgnoreParenCasts() const {
+ return const_cast<Expr *>(this)->IgnoreParenCasts();
}
- /// Ignore parentheses and lvalue casts. Strip off any ParenExpr and
- /// CastExprs that represent lvalue casts, returning their operand.
+ /// Skip conversion operators. If this Expr is a call to a conversion
+ /// operator, return the argument.
+ 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.
Expr *IgnoreParenLValueCasts() LLVM_READONLY;
-
- const Expr *IgnoreParenLValueCasts() const LLVM_READONLY {
- return const_cast<Expr*>(this)->IgnoreParenLValueCasts();
- }
-
- /// IgnoreParenNoopCasts - Ignore parentheses and casts that do not change the
- /// value (including ptr->int casts of the same size). Strip off any
- /// ParenExpr or CastExprs, returning their operand.
- Expr *IgnoreParenNoopCasts(ASTContext &Ctx) LLVM_READONLY;
-
- /// Ignore parentheses and derived-to-base casts.
+ 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.
+ 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).
Expr *ignoreParenBaseCasts() LLVM_READONLY;
-
- const Expr *ignoreParenBaseCasts() const LLVM_READONLY {
- return const_cast<Expr*>(this)->ignoreParenBaseCasts();
+ const Expr *ignoreParenBaseCasts() const {
+ return const_cast<Expr *>(this)->ignoreParenBaseCasts();
}
/// Determine whether this expression is a default function argument.
@@ -823,24 +866,6 @@
/// Whether this expression is an implicit reference to 'this' in C++.
bool isImplicitCXXThis() const;
- const Expr *IgnoreImpCasts() const LLVM_READONLY {
- return const_cast<Expr*>(this)->IgnoreImpCasts();
- }
- const Expr *IgnoreParens() const LLVM_READONLY {
- return const_cast<Expr*>(this)->IgnoreParens();
- }
- const Expr *IgnoreParenCasts() const LLVM_READONLY {
- return const_cast<Expr*>(this)->IgnoreParenCasts();
- }
- /// Strip off casts, but keep parentheses.
- const Expr *IgnoreCasts() const LLVM_READONLY {
- return const_cast<Expr*>(this)->IgnoreCasts();
- }
-
- const Expr *IgnoreParenNoopCasts(ASTContext &Ctx) const LLVM_READONLY {
- return const_cast<Expr*>(this)->IgnoreParenNoopCasts(Ctx);
- }
-
static bool hasAnyTypeDependentArguments(ArrayRef<Expr *> Exprs);
/// For an expression of class type or pointer to class type,
@@ -3165,18 +3190,6 @@
friend class CastExpr;
};
-inline Expr *Expr::IgnoreImpCasts() {
- Expr *e = this;
- while (true)
- if (ImplicitCastExpr *ice = dyn_cast<ImplicitCastExpr>(e))
- e = ice->getSubExpr();
- else if (FullExpr *fe = dyn_cast<FullExpr>(e))
- e = fe->getSubExpr();
- else
- break;
- return e;
-}
-
/// ExplicitCastExpr - An explicit cast written in the source
/// code.
///
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
