li.zhe.hua updated this revision to Diff 426987.
li.zhe.hua added a comment.
Forgot to add a file... let's try this again...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124807/new/
https://reviews.llvm.org/D124807
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/Transfer.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1152,4 +1152,39 @@
});
}
+// FIXME: Remove this test once it provides no additional test coverage.
+TEST_F(FlowConditionTest, DoesNotAssertForImplicitCastToBool) {
+ std::string Code = R"(
+ void target(int *Ptr) {
+ bool Foo = false;
+ if (Ptr) {
+ Foo = true;
+ /*[[p1]]*/
+ }
+
+ (void)0;
+ /*[[p2]]*/
+ }
+ )";
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
+
+ const Environment &Env1 = Results[1].second.Env;
+ auto &FooVal1 =
+ *cast<BoolValue>(Env1.getValue(*FooDecl, SkipPast::Reference));
+ EXPECT_TRUE(Env1.flowConditionImplies(FooVal1));
+
+ const Environment &Env2 = Results[0].second.Env;
+ auto &FooVal2 =
+ *cast<BoolValue>(Env2.getValue(*FooDecl, SkipPast::Reference));
+ EXPECT_FALSE(Env2.flowConditionImplies(FooVal2));
+ });
+}
+
} // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -77,26 +77,26 @@
: StmtToEnv(StmtToEnv), Env(Env), BlockSuccIdx(BlockSuccIdx) {}
void VisitIfStmt(const IfStmt *S) {
- auto *Cond = S->getCond()->IgnoreParenImpCasts();
+ auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
assert(Cond != nullptr);
extendFlowCondition(*Cond);
}
void VisitWhileStmt(const WhileStmt *S) {
- auto *Cond = S->getCond()->IgnoreParenImpCasts();
+ auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
assert(Cond != nullptr);
extendFlowCondition(*Cond);
}
void VisitBinaryOperator(const BinaryOperator *S) {
assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
- auto *LHS = S->getLHS()->IgnoreParenImpCasts();
+ auto *LHS = ignoreExprWithCleanups(S->getLHS())->IgnoreParens();
assert(LHS != nullptr);
extendFlowCondition(*LHS);
}
void VisitConditionalOperator(const ConditionalOperator *S) {
- auto *Cond = S->getCond()->IgnoreParenImpCasts();
+ auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
assert(Cond != nullptr);
extendFlowCondition(*Cond);
}
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -33,7 +33,7 @@
namespace clang {
namespace dataflow {
-static const Expr *skipExprWithCleanups(const Expr *E) {
+const Expr *ignoreExprWithCleanups(const Expr *E) {
if (auto *C = dyn_cast_or_null<ExprWithCleanups>(E))
return C->getSubExpr();
return E;
@@ -155,9 +155,7 @@
return;
}
- // The CFG does not contain `ParenExpr` as top-level statements in basic
- // blocks, however sub-expressions can still be of that type.
- InitExpr = skipExprWithCleanups(D.getInit()->IgnoreParens());
+ InitExpr = ignoreExprWithCleanups(D.getInit());
assert(InitExpr != nullptr);
if (D.getType()->isReferenceType()) {
@@ -190,10 +188,7 @@
}
void VisitImplicitCastExpr(const ImplicitCastExpr *S) {
- // The CFG does not contain `ParenExpr` as top-level statements in basic
- // blocks, however sub-expressions can still be of that type.
- assert(S->getSubExpr() != nullptr);
- const Expr *SubExpr = S->getSubExpr()->IgnoreParens();
+ const Expr *SubExpr = S->getSubExpr();
assert(SubExpr != nullptr);
switch (S->getCastKind()) {
@@ -252,10 +247,7 @@
}
void VisitUnaryOperator(const UnaryOperator *S) {
- // The CFG does not contain `ParenExpr` as top-level statements in basic
- // blocks, however sub-expressions can still be of that type.
- assert(S->getSubExpr() != nullptr);
- const Expr *SubExpr = S->getSubExpr()->IgnoreParens();
+ const Expr *SubExpr = S->getSubExpr();
assert(SubExpr != nullptr);
switch (S->getOpcode()) {
@@ -444,9 +436,6 @@
void VisitCXXFunctionalCastExpr(const CXXFunctionalCastExpr *S) {
if (S->getCastKind() == CK_ConstructorConversion) {
- // The CFG does not contain `ParenExpr` as top-level statements in basic
- // blocks, however sub-expressions can still be of that type.
- assert(S->getSubExpr() != nullptr);
const Expr *SubExpr = S->getSubExpr();
assert(SubExpr != nullptr);
@@ -604,7 +593,7 @@
};
void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env) {
- assert(!isa<ParenExpr>(&S));
+ assert(!(isa<ParenExpr, ExprWithCleanups>(&S)));
TransferVisitor(StmtToEnv, Env).Visit(&S);
}
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -15,6 +15,7 @@
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
#include "clang/AST/Type.h"
#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
#include "clang/Analysis/FlowSensitive/StorageLocation.h"
@@ -342,6 +343,7 @@
}
StorageLocation &Environment::createStorageLocation(const Expr &E) {
+ assert(!isa<ExprWithCleanups>(&E));
// Evaluated expressions are always assigned the same storage locations to
// ensure that the environment stabilizes across loop iterations. Storage
// locations for evaluated expressions are stored in the analysis context.
@@ -364,12 +366,14 @@
}
void Environment::setStorageLocation(const Expr &E, StorageLocation &Loc) {
+ assert(!isa<ExprWithCleanups>(&E));
assert(ExprToLoc.find(&E) == ExprToLoc.end());
ExprToLoc[&E] = &Loc;
}
StorageLocation *Environment::getStorageLocation(const Expr &E,
SkipPast SP) const {
+ assert(!isa<ExprWithCleanups>(&E));
// FIXME: Add a test with parens.
auto It = ExprToLoc.find(E.IgnoreParens());
return It == ExprToLoc.end() ? nullptr : &skip(*It->second, SP);
Index: clang/include/clang/Analysis/FlowSensitive/Transfer.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/Transfer.h
+++ clang/include/clang/Analysis/FlowSensitive/Transfer.h
@@ -35,9 +35,19 @@
///
/// Requirements:
///
-/// The type of `S` must not be `ParenExpr`.
+/// `S` must not be `ParenExpr` or `ExprWithCleanups`.
void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env);
+/// Skip past a `ExprWithCleanups` which might surround `E`. Returns null if `E`
+/// is null.
+///
+/// The CFG omits `ExprWithCleanups` nodes (as it does with `ParenExpr`), and so
+/// the transfer function doesn't accept them as valid input. Manual traversal
+/// of the AST should skip and unwrap any `ExprWithCleanups` it might expect to
+/// see. They are safe to skip, as the CFG will emit calls to destructors as
+/// appropriate.
+const Expr *ignoreExprWithCleanups(const Expr *E);
+
} // namespace dataflow
} // namespace clang
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -172,6 +172,10 @@
/// Creates a storage location for `E`. Does not assign the returned storage
/// location to `E` in the environment. Does not assign a value to the
/// returned storage location in the environment.
+ ///
+ /// Requirements:
+ ///
+ /// `E` must not be a `ExprWithCleanups`.
StorageLocation &createStorageLocation(const Expr &E);
/// Assigns `Loc` as the storage location of `D` in the environment.
@@ -191,11 +195,16 @@
/// Requirements:
///
/// `E` must not be assigned a storage location in the environment.
+ /// `E` must not be a `ExprWithCleanups`.
void setStorageLocation(const Expr &E, StorageLocation &Loc);
/// Returns the storage location assigned to `E` in the environment, applying
/// the `SP` policy for skipping past indirections, or null if `E` isn't
/// assigned a storage location in the environment.
+ ///
+ /// Requirements:
+ ///
+ /// `E` must not be a `ExprWithCleanups`.
StorageLocation *getStorageLocation(const Expr &E, SkipPast SP) const;
/// Returns the storage location assigned to the `this` pointee in the
@@ -226,6 +235,12 @@
/// Equivalent to `getValue(getStorageLocation(E, SP), SkipPast::None)` if `E`
/// is assigned a storage location in the environment, otherwise returns null.
+ ///
+ /// Requirements:
+ ///
+ /// `E` must not be a `ExprWithCleanups`.
+ ///
+ /// FIXME: `Environment` should ignore any `ExprWithCleanups` it sees.
Value *getValue(const Expr &E, SkipPast SP) const;
/// Transfers ownership of `Loc` to the analysis context and returns a
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
