[clang] 7587065 - Revert "[clang][dataflow] Analyze constructor bodies"
Evgenii Stepanov via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 10 14:30:04 PDT 2022
Author: Evgenii Stepanov
Date: 2022-08-10T14:21:56-07:00
New Revision: 75870650433de9e7bfbe86adc1bef2f2a23fe7a3
URL: https://github.com/llvm/llvm-project/commit/75870650433de9e7bfbe86adc1bef2f2a23fe7a3
DIFF: https://github.com/llvm/llvm-project/commit/75870650433de9e7bfbe86adc1bef2f2a23fe7a3.diff
LOG: Revert "[clang][dataflow] Analyze constructor bodies"
https://lab.llvm.org/buildbot/#/builders/74/builds/12713
This reverts commit 000c8fef86abb7f056cbea2de99f21dca4b81bf8.
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 8c169005846ef..1b154010bf365 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -135,7 +135,7 @@ class Environment {
///
/// Requirements:
///
- /// The callee of `Call` must be a `FunctionDecl`.
+ /// The callee of `Call` must be a `FunctionDecl` with a body.
///
/// The body of the callee must not reference globals.
///
@@ -143,7 +143,6 @@ class Environment {
///
/// Each argument of `Call` must already have a `StorageLocation`.
Environment pushCall(const CallExpr *Call) const;
- Environment pushCall(const CXXConstructExpr *Call) const;
/// Moves gathered information back into `this` from a `CalleeEnv` created via
/// `pushCall`.
@@ -382,12 +381,6 @@ class Environment {
StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const;
const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const;
- /// Shared implementation of `pushCall` overloads. Note that unlike
- /// `pushCall`, this member is invoked on the environment of the callee, not
- /// of the caller.
- void pushCallInternal(const FunctionDecl *FuncDecl,
- ArrayRef<const Expr *> Args);
-
// `DACtx` is not null and not owned by this object.
DataflowAnalysisContext *DACtx;
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index e4af68e53e14e..16c83cad9d9e3 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -207,68 +207,52 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
Environment Environment::pushCall(const CallExpr *Call) const {
Environment Env(*this);
-
// FIXME: Support references here.
- Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);
-
- if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
- if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
- Env.ThisPointeeLoc = getStorageLocation(*Arg, SkipPast::Reference);
- }
- }
-
- Env.pushCallInternal(Call->getDirectCallee(),
- llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
+ Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference);
- return Env;
-}
-
-Environment Environment::pushCall(const CXXConstructExpr *Call) const {
- Environment Env(*this);
-
- // FIXME: Support references here.
- Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference);
+ const auto *FuncDecl = Call->getDirectCallee();
+ assert(FuncDecl != nullptr);
- Env.ThisPointeeLoc = Env.ReturnLoc;
-
- Env.pushCallInternal(Call->getConstructor(),
- llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs()));
-
- return Env;
-}
-
-void Environment::pushCallInternal(const FunctionDecl *FuncDecl,
- ArrayRef<const Expr *> Args) {
- setDeclCtx(FuncDecl);
+ Env.setDeclCtx(FuncDecl);
// FIXME: In order to allow the callee to reference globals, we probably need
// to call `initGlobalVars` here in some way.
+ if (const auto *MethodCall = dyn_cast<CXXMemberCallExpr>(Call)) {
+ if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) {
+ Env.ThisPointeeLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
+ }
+ }
+
auto ParamIt = FuncDecl->param_begin();
+ auto ArgIt = Call->arg_begin();
+ auto ArgEnd = Call->arg_end();
// FIXME: Parameters don't always map to arguments 1:1; examples include
// overloaded operators implemented as member functions, and parameter packs.
- for (unsigned ArgIndex = 0; ArgIndex < Args.size(); ++ParamIt, ++ArgIndex) {
+ for (; ArgIt != ArgEnd; ++ParamIt, ++ArgIt) {
assert(ParamIt != FuncDecl->param_end());
- const Expr *Arg = Args[ArgIndex];
- auto *ArgLoc = getStorageLocation(*Arg, SkipPast::Reference);
+ const Expr *Arg = *ArgIt;
+ auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
assert(ArgLoc != nullptr);
const VarDecl *Param = *ParamIt;
- auto &Loc = createStorageLocation(*Param);
- setStorageLocation(*Param, Loc);
+ auto &Loc = Env.createStorageLocation(*Param);
+ Env.setStorageLocation(*Param, Loc);
QualType ParamType = Param->getType();
if (ParamType->isReferenceType()) {
- auto &Val = takeOwnership(std::make_unique<ReferenceValue>(*ArgLoc));
- setValue(Loc, Val);
- } else if (auto *ArgVal = getValue(*ArgLoc)) {
- setValue(Loc, *ArgVal);
- } else if (Value *Val = createValue(ParamType)) {
- setValue(Loc, *Val);
+ auto &Val = Env.takeOwnership(std::make_unique<ReferenceValue>(*ArgLoc));
+ Env.setValue(Loc, Val);
+ } else if (auto *ArgVal = Env.getValue(*ArgLoc)) {
+ Env.setValue(Loc, *ArgVal);
+ } else if (Value *Val = Env.createValue(ParamType)) {
+ Env.setValue(Loc, *Val);
}
}
+
+ return Env;
}
void Environment::popCall(const Environment &CalleeEnv) {
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 2f05b08fe9bc2..430dc8d5faa97 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -444,8 +444,6 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
Env.setStorageLocation(*S, Loc);
if (Value *Val = Env.createValue(S->getType()))
Env.setValue(Loc, *Val);
-
- transferInlineCall(S, ConstructorDecl);
}
void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *S) {
@@ -528,7 +526,45 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
return;
Env.setStorageLocation(*S, *ArgLoc);
} else if (const FunctionDecl *F = S->getDirectCallee()) {
- transferInlineCall(S, F);
+ // This case is for context-sensitive analysis.
+ if (!Options.ContextSensitive)
+ return;
+
+ const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
+ if (!CFCtx)
+ return;
+
+ // FIXME: We don't support context-sensitive analysis of recursion, so
+ // we should return early here if `F` is the same as the `FunctionDecl`
+ // holding `S` itself.
+
+ auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
+
+ // Note that it is important for the storage location of `S` to be set
+ // before `pushCall`, because the latter uses it to set the storage
+ // location for `return`.
+ auto &ReturnLoc = Env.createStorageLocation(*S);
+ Env.setStorageLocation(*S, ReturnLoc);
+ auto CalleeEnv = Env.pushCall(S);
+
+ // FIXME: Use the same analysis as the caller for the callee. Note,
+ // though, that doing so would require support for changing the analysis's
+ // ASTContext.
+ assert(
+ CFCtx->getDecl() != nullptr &&
+ "ControlFlowContexts in the environment should always carry a decl");
+ auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(),
+ DataflowAnalysisOptions());
+
+ auto BlockToOutputState =
+ dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
+ assert(BlockToOutputState);
+ assert(ExitBlock < BlockToOutputState->size());
+
+ auto ExitState = (*BlockToOutputState)[ExitBlock];
+ assert(ExitState);
+
+ Env.popCall(ExitState->Env);
}
}
@@ -657,51 +693,6 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
return Env.makeAtomicBoolValue();
}
- // If context sensitivity is enabled, try to analyze the body of the callee
- // `F` of `S`. The type `E` must be either `CallExpr` or `CXXConstructExpr`.
- template <typename E>
- void transferInlineCall(const E *S, const FunctionDecl *F) {
- if (!Options.ContextSensitive)
- return;
-
- const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
- if (!CFCtx)
- return;
-
- // FIXME: We don't support context-sensitive analysis of recursion, so
- // we should return early here if `F` is the same as the `FunctionDecl`
- // holding `S` itself.
-
- auto ExitBlock = CFCtx->getCFG().getExit().getBlockID();
-
- if (const auto *NonConstructExpr = dyn_cast<CallExpr>(S)) {
- // Note that it is important for the storage location of `S` to be set
- // before `pushCall`, because the latter uses it to set the storage
- // location for `return`.
- auto &ReturnLoc = Env.createStorageLocation(*S);
- Env.setStorageLocation(*S, ReturnLoc);
- }
- auto CalleeEnv = Env.pushCall(S);
-
- // FIXME: Use the same analysis as the caller for the callee. Note,
- // though, that doing so would require support for changing the analysis's
- // ASTContext.
- assert(CFCtx->getDecl() != nullptr &&
- "ControlFlowContexts in the environment should always carry a decl");
- auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(),
- DataflowAnalysisOptions());
-
- auto BlockToOutputState =
- dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv);
- assert(BlockToOutputState);
- assert(ExitBlock < BlockToOutputState->size());
-
- auto ExitState = (*BlockToOutputState)[ExitBlock];
- assert(ExitState);
-
- Env.popCall(ExitState->Env);
- }
-
const StmtToEnvMap &StmtToEnv;
Environment &Env;
TransferOptions Options;
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index af06021abccfd..ebca4b3ea7f15 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -4368,106 +4368,4 @@ TEST(TransferTest, ContextSensitiveMethodGetterAndSetter) {
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
}
-TEST(TransferTest, ContextSensitiveConstructorBody) {
- std::string Code = R"(
- class MyClass {
- public:
- MyClass() { MyField = true; }
-
- bool MyField;
- };
-
- void target() {
- MyClass MyObj;
- bool Foo = MyObj.MyField;
- // [[p]]
- }
- )";
- runDataflow(Code,
- [](llvm::ArrayRef<
- std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
- Results,
- ASTContext &ASTCtx) {
- ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
- const Environment &Env = Results[0].second.Env;
-
- const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
- ASSERT_THAT(FooDecl, NotNull());
-
- auto &FooVal =
- *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
- EXPECT_TRUE(Env.flowConditionImplies(FooVal));
- },
- {/*.ApplyBuiltinTransfer=*/true,
- /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
-}
-
-TEST(TransferTest, ContextSensitiveConstructorInitializer) {
- std::string Code = R"(
- class MyClass {
- public:
- MyClass() : MyField(true) {}
-
- bool MyField;
- };
-
- void target() {
- MyClass MyObj;
- bool Foo = MyObj.MyField;
- // [[p]]
- }
- )";
- runDataflow(Code,
- [](llvm::ArrayRef<
- std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
- Results,
- ASTContext &ASTCtx) {
- ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
- const Environment &Env = Results[0].second.Env;
-
- const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
- ASSERT_THAT(FooDecl, NotNull());
-
- auto &FooVal =
- *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
- EXPECT_TRUE(Env.flowConditionImplies(FooVal));
- },
- {/*.ApplyBuiltinTransfer=*/true,
- /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
-}
-
-TEST(TransferTest, ContextSensitiveConstructorDefault) {
- std::string Code = R"(
- class MyClass {
- public:
- MyClass() = default;
-
- bool MyField = true;
- };
-
- void target() {
- MyClass MyObj;
- bool Foo = MyObj.MyField;
- // [[p]]
- }
- )";
- runDataflow(Code,
- [](llvm::ArrayRef<
- std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
- Results,
- ASTContext &ASTCtx) {
- ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
- const Environment &Env = Results[0].second.Env;
-
- const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
- ASSERT_THAT(FooDecl, NotNull());
-
- auto &FooVal =
- *cast<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
- EXPECT_TRUE(Env.flowConditionImplies(FooVal));
- },
- {/*.ApplyBuiltinTransfer=*/true,
- /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
-}
-
} // namespace
More information about the cfe-commits
mailing list