mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150352

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.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
@@ -68,7 +68,7 @@
   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>());
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -39,10 +39,11 @@
 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 = {
       "-fsyntax-only", "-fno-delayed-template-parsing",
@@ -61,13 +62,21 @@
   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 +2543,34 @@
       });
 }
 
-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) {
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -68,7 +68,12 @@
 }
 
 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 @@
   // 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 @@
 
   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
Index: clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
+++ clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
@@ -32,7 +32,13 @@
 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);
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to