[clang] 1a42f79 - [clang][dataflow] Don't analyze templated declarations.
Martin Braenne via cfe-commits
cfe-commits at lists.llvm.org
Mon May 15 04:05:01 PDT 2023
Author: Martin Braenne
Date: 2023-05-15T11:04:51Z
New Revision: 1a42f795587b9d57291d009989aace6efd0a7a7f
URL: https://github.com/llvm/llvm-project/commit/1a42f795587b9d57291d009989aace6efd0a7a7f
DIFF: https://github.com/llvm/llvm-project/commit/1a42f795587b9d57291d009989aace6efd0a7a7f.diff
LOG: [clang][dataflow] Don't analyze templated declarations.
Attempting to analyze templated code doesn't have a good cost-benefit ratio. We
have so far done a best-effort attempt at this, but maintaining this support has
an ongoing high maintenance cost because the AST for templates can violate a lot
of the invariants that otherwise hold for the AST of concrete code. As just one
example, in concrete code the operand of a UnaryOperator '*' is always a prvalue
(https://godbolt.org/z/s3e5xxMd1), but in templates this isn't true
(https://godbolt.org/z/6W9xxGvoM).
Further rationale for not analyzing templates:
* The semantics of a template itself are weakly defined; semantics can depend
strongly on the concrete template arguments. Analyzing the template itself (as
opposed to an instantiation) therefore has limited value.
* Analyzing templates requires a lot of special-case code that isn't necessary
for concrete code because dependent types are hard to deal with and the AST
violates invariants that otherwise hold for concrete code (see above).
* There's precedent in that neither Clang Static Analyzer nor the flow-sensitive
warnings in Clang (such as uninitialized variables) support analyzing
templates.
Reviewed By: gribozavr2, xazax.hun
Differential Revision: https://reviews.llvm.org/D150352
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h b/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
index 3495bdfc538cb..b51e2cb23634d 100644
--- a/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
@@ -32,7 +32,13 @@ namespace dataflow {
class ControlFlowContext {
public:
/// Builds a ControlFlowContext from an AST node. `D` is the function in which
- /// `S` resides and must not be null.
+ /// `S` resides. `D.isTemplated()` must be false.
+ static llvm::Expected<ControlFlowContext> build(const Decl &D, Stmt &S,
+ ASTContext &C);
+
+ /// Builds a ControlFlowContext from an AST node. `D` is the function in which
+ /// `S` resides. `D` must not be null and `D->isTemplated()` must be false.
+ LLVM_DEPRECATED("Use the version that takes a const Decl & instead", "")
static llvm::Expected<ControlFlowContext> build(const Decl *D, Stmt &S,
ASTContext &C);
diff --git a/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp b/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
index 5520633da68ae..4556787d10a8e 100644
--- a/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -68,7 +68,12 @@ static llvm::BitVector findReachableBlocks(const CFG &Cfg) {
}
llvm::Expected<ControlFlowContext>
-ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
+ControlFlowContext::build(const Decl &D, Stmt &S, ASTContext &C) {
+ if (D.isTemplated())
+ return llvm::createStringError(
+ std::make_error_code(std::errc::invalid_argument),
+ "Cannot analyze templated declarations");
+
CFG::BuildOptions Options;
Options.PruneTriviallyFalseEdges = true;
Options.AddImplicitDtors = true;
@@ -79,7 +84,7 @@ ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
// Ensure that all sub-expressions in basic blocks are evaluated.
Options.setAllAlwaysAdd();
- auto Cfg = CFG::buildCFG(D, &S, &C, Options);
+ auto Cfg = CFG::buildCFG(&D, &S, &C, Options);
if (Cfg == nullptr)
return llvm::createStringError(
std::make_error_code(std::errc::invalid_argument),
@@ -90,9 +95,19 @@ ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
llvm::BitVector BlockReachable = findReachableBlocks(*Cfg);
- return ControlFlowContext(D, std::move(Cfg), std::move(StmtToBlock),
+ return ControlFlowContext(&D, std::move(Cfg), std::move(StmtToBlock),
std::move(BlockReachable));
}
+llvm::Expected<ControlFlowContext>
+ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
+ if (D == nullptr)
+ return llvm::createStringError(
+ std::make_error_code(std::errc::invalid_argument),
+ "Declaration must not be null");
+
+ return build(*D, S, C);
+}
+
} // namespace dataflow
} // namespace clang
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index 5dd390e962d82..73428ac250ad3 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -211,7 +211,7 @@ DataflowAnalysisContext::getControlFlowContext(const FunctionDecl *F) {
return &It->second;
if (Stmt *Body = F->getBody()) {
- auto CFCtx = ControlFlowContext::build(F, *Body, F->getASTContext());
+ auto CFCtx = ControlFlowContext::build(*F, *Body, F->getASTContext());
// FIXME: Handle errors.
assert(CFCtx);
auto Result = FunctionContexts.insert({F, std::move(*CFCtx)});
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 142dd2f21a5c6..95810a47fc632 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -424,9 +424,8 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
switch (S->getOpcode()) {
case UO_Deref: {
- // Skip past a reference to handle dereference of a dependent pointer.
- const auto *SubExprVal = cast_or_null<PointerValue>(
- Env.getValue(*SubExpr, SkipPast::Reference));
+ const auto *SubExprVal =
+ cast_or_null<PointerValue>(Env.getValue(*SubExpr, SkipPast::None));
if (SubExprVal == nullptr)
break;
diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index e1a56b049f6de..ff7d27d6540cc 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -242,7 +242,7 @@ checkDataflow(AnalysisInputs<AnalysisT> AI,
// Build the control flow graph for the target function.
auto MaybeCFCtx =
- ControlFlowContext::build(Target, *Target->getBody(), Context);
+ ControlFlowContext::build(*Target, *Target->getBody(), Context);
if (!MaybeCFCtx) return MaybeCFCtx.takeError();
auto &CFCtx = *MaybeCFCtx;
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 54d3c5751b23b..7c2a7014354aa 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -39,12 +39,16 @@ using ::testing::UnorderedElementsAre;
using BuiltinOptions = DataflowAnalysisContext::Options;
template <typename Matcher>
-void runDataflow(llvm::StringRef Code, Matcher Match,
- DataflowAnalysisOptions Options,
- LangStandard::Kind Std = LangStandard::lang_cxx17,
- llvm::StringRef TargetFun = "target") {
+llvm::Error
+runDataflowReturnError(llvm::StringRef Code, Matcher Match,
+ DataflowAnalysisOptions Options,
+ LangStandard::Kind Std = LangStandard::lang_cxx17,
+ llvm::StringRef TargetFun = "target") {
using ast_matchers::hasName;
llvm::SmallVector<std::string, 3> ASTBuildArgs = {
+ // -fnodelayed-template-parsing is the default everywhere but on Windows.
+ // Set it explicitly so that tests behave the same on Windows as on other
+ // platforms.
"-fsyntax-only", "-fno-delayed-template-parsing",
"-std=" +
std::string(LangStandard::getLangStandardForKind(Std).getName())};
@@ -61,13 +65,21 @@ void runDataflow(llvm::StringRef Code, Matcher Match,
AI.ASTBuildArgs = ASTBuildArgs;
if (Options.BuiltinOpts)
AI.BuiltinOptions = *Options.BuiltinOpts;
+ return checkDataflow<NoopAnalysis>(
+ std::move(AI),
+ /*VerifyResults=*/
+ [&Match](
+ const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ const AnalysisOutputs &AO) { Match(Results, AO.ASTCtx); });
+}
+
+template <typename Matcher>
+void runDataflow(llvm::StringRef Code, Matcher Match,
+ DataflowAnalysisOptions Options,
+ LangStandard::Kind Std = LangStandard::lang_cxx17,
+ llvm::StringRef TargetFun = "target") {
ASSERT_THAT_ERROR(
- checkDataflow<NoopAnalysis>(
- std::move(AI),
- /*VerifyResults=*/
- [&Match](const llvm::StringMap<DataflowAnalysisState<NoopLattice>>
- &Results,
- const AnalysisOutputs &AO) { Match(Results, AO.ASTCtx); }),
+ runDataflowReturnError(Code, Match, Options, Std, TargetFun),
llvm::Succeeded());
}
@@ -2534,31 +2546,34 @@ TEST(TransferTest, AddrOfReference) {
});
}
-TEST(TransferTest, DerefDependentPtr) {
+TEST(TransferTest, CannotAnalyzeFunctionTemplate) {
std::string Code = R"(
template <typename T>
- void target(T *Foo) {
- T &Bar = *Foo;
- /*[[p]]*/
- }
+ void target() {}
)";
- runDataflow(
- Code,
- [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
- ASTContext &ASTCtx) {
- ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
- const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
-
- const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
- ASSERT_THAT(FooDecl, NotNull());
-
- const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
- ASSERT_THAT(BarDecl, NotNull());
+ ASSERT_THAT_ERROR(
+ runDataflowReturnError(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {},
+ {BuiltinOptions()}),
+ llvm::FailedWithMessage("Cannot analyze templated declarations"));
+}
- const auto *FooVal = cast<PointerValue>(Env.getValue(*FooDecl));
- const auto *BarLoc = Env.getStorageLocation(*BarDecl);
- EXPECT_EQ(BarLoc, &FooVal->getPointeeLoc());
- });
+TEST(TransferTest, CannotAnalyzeMethodOfClassTemplate) {
+ std::string Code = R"(
+ template <typename T>
+ struct A {
+ void target() {}
+ };
+ )";
+ ASSERT_THAT_ERROR(
+ runDataflowReturnError(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {},
+ {BuiltinOptions()}),
+ llvm::FailedWithMessage("Cannot analyze templated declarations"));
}
TEST(TransferTest, VarDeclInitAssignConditionalOperator) {
diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 83b9f33493d0b..30aef86e7a26e 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -68,7 +68,7 @@ runAnalysis(llvm::StringRef Code, AnalysisT (*MakeAnalysis)(ASTContext &)) {
assert(Body != nullptr);
auto CFCtx = llvm::cantFail(
- ControlFlowContext::build(nullptr, *Body, AST->getASTContext()));
+ ControlFlowContext::build(*Func, *Body, AST->getASTContext()));
AnalysisT Analysis = MakeAnalysis(AST->getASTContext());
DataflowAnalysisContext DACtx(std::make_unique<WatchedLiteralsSolver>());
More information about the cfe-commits
mailing list