ymandel updated this revision to Diff 544367.
ymandel marked 4 inline comments as done.
ymandel added a comment.

address comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156254/new/

https://reviews.llvm.org/D156254

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/include/clang/Analysis/FlowSensitive/NoopAnalysis.h
  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
@@ -47,6 +47,7 @@
 using namespace ast_matchers;
 using llvm::IsStringMapEntry;
 using ::testing::DescribeMatcher;
+using ::testing::ElementsAre;
 using ::testing::IsEmpty;
 using ::testing::NotNull;
 using ::testing::Test;
@@ -84,6 +85,30 @@
   EXPECT_TRUE(BlockStates[1].has_value());
 }
 
+// Basic test that `diagnoseFunction` calls the Diagnoser function for the
+// number of elements expected.
+TEST(DataflowAnalysisTest, DiagnoseFunctionDiagnoserCalledOnEachElement) {
+  std::string Code = R"(void target() { int x = 0; ++x; })";
+  std::unique_ptr<ASTUnit> AST =
+      tooling::buildASTFromCodeWithArgs(Code, {"-std=c++11"});
+
+  auto *Func =
+      cast<FunctionDecl>(findValueDecl(AST->getASTContext(), "target"));
+  auto Diagnoser = [](const CFGElement &Elt, ASTContext &,
+                      const TransferStateForDiagnostics<NoopLattice> &) {
+    std::vector<std::string> Diagnostics(1);
+    llvm::raw_string_ostream OS(Diagnostics.front());
+    Elt.dumpToStream(OS);
+    return Diagnostics;
+  };
+  auto Result = diagnoseFunction<NoopAnalysis, std::string>(
+      *Func, AST->getASTContext(), Diagnoser);
+  // `diagnoseFunction` provides no guarantees about the order in which elements
+  // are visited, so we use `UnorderedElementsAre`.
+  EXPECT_THAT_EXPECTED(Result, llvm::HasValue(UnorderedElementsAre(
+                                   "0\n", "int x = 0;\n", "x\n", "++x\n")));
+}
+
 struct NonConvergingLattice {
   int State;
 
Index: clang/include/clang/Analysis/FlowSensitive/NoopAnalysis.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/NoopAnalysis.h
+++ clang/include/clang/Analysis/FlowSensitive/NoopAnalysis.h
@@ -24,6 +24,9 @@
 
 class NoopAnalysis : public DataflowAnalysis<NoopAnalysis, NoopLattice> {
 public:
+  NoopAnalysis(ASTContext &Context)
+      : DataflowAnalysis<NoopAnalysis, NoopLattice>(Context) {}
+
   /// Deprecated. Use the `DataflowAnalysisOptions` constructor instead.
   NoopAnalysis(ASTContext &Context, bool ApplyBuiltinTransfer)
       : DataflowAnalysis<NoopAnalysis, NoopLattice>(Context,
Index: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
+++ clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
@@ -48,6 +48,10 @@
 };
 
 /// A read-only version of TransferState.
+///
+/// FIXME: this type is being used as a general (typed) view type for untyped
+/// dataflow analysis state, rather than strictly for transfer-function
+/// purposes. Move it (and rename it) to DataflowAnalysis.h.
 template <typename LatticeT> struct TransferStateForDiagnostics {
   TransferStateForDiagnostics(const LatticeT &Lattice, const Environment &Env)
       : Lattice(Lattice), Env(Env) {}
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -25,9 +25,13 @@
 #include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "llvm/ADT/Any.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 
 namespace clang {
@@ -238,6 +242,60 @@
   return std::move(BlockStates);
 }
 
+/// Runs a dataflow analysis over the given function and then runs `Diagnoser`
+/// over the results. Returns a list of diagnostics for `FuncDecl` or an
+/// error. Currently, errors can occur (at least) because the analysis requires
+/// too many iterations over the CFG or the SAT solver times out.
+///
+/// The default value of `MaxSATIterations` was chosen based on the following
+/// observations:
+/// - Non-pathological calls to the solver typically require only a few hundred
+///   iterations.
+/// - This limit is still low enough to keep runtimes acceptable (on typical
+///   machines) in cases where we hit the limit.
+template <typename AnalysisT, typename Diagnostic>
+llvm::Expected<std::vector<Diagnostic>> diagnoseFunction(
+    const FunctionDecl &FuncDecl, ASTContext &ASTCtx,
+    llvm::function_ref<std::vector<Diagnostic>(
+        const CFGElement &, ASTContext &,
+        const TransferStateForDiagnostics<typename AnalysisT::Lattice> &)>
+        Diagnoser,
+    std::int64_t MaxSATIterations = 1'000'000'000) {
+  llvm::Expected<ControlFlowContext> Context =
+      ControlFlowContext::build(FuncDecl);
+  if (!Context)
+    return Context.takeError();
+
+  auto OwnedSolver = std::make_unique<WatchedLiteralsSolver>(MaxSATIterations);
+  const WatchedLiteralsSolver *Solver = OwnedSolver.get();
+  DataflowAnalysisContext AnalysisContext(std::move(OwnedSolver));
+  Environment Env(AnalysisContext, FuncDecl);
+  AnalysisT Analysis(ASTCtx);
+  std::vector<Diagnostic> Diagnostics;
+  if (llvm::Error Err =
+          runTypeErasedDataflowAnalysis(
+              *Context, Analysis, Env,
+              [&ASTCtx, &Diagnoser, &Diagnostics](
+                  const CFGElement &Elt,
+                  const TypeErasedDataflowAnalysisState &State) mutable {
+                auto EltDiagnostics = Diagnoser(
+                    Elt, ASTCtx,
+                    TransferStateForDiagnostics<typename AnalysisT::Lattice>(
+                        llvm::any_cast<const typename AnalysisT::Lattice &>(
+                            State.Lattice.Value),
+                        State.Env));
+                llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
+              })
+              .takeError())
+    return Err;
+
+  if (Solver->reachedLimit())
+    return llvm::createStringError(llvm::errc::interrupted,
+                                   "SAT solver timed out");
+
+  return Diagnostics;
+}
+
 /// Abstract base class for dataflow "models": reusable analysis components that
 /// model a particular aspect of program semantics in the `Environment`. For
 /// example, a model may capture a type and its related functions.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to