[clang] b295607 - [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

[Kirstóf Umann via cfe-commits]

Author: Kirstóf Umann
Date: 2020-07-06T14:51:37+02:00
New Revision: b2956076976cd0f435c46257f7cf9a0e5376d0fa

URL: https://github.com/llvm/llvm-project/commit/b2956076976cd0f435c46257f7cf9a0e5376d0fa
DIFF: https://github.com/llvm/llvm-project/commit/b2956076976cd0f435c46257f7cf9a0e5376d0fa.diff

LOG: [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

The thrilling conclusion to the barrage of patches I uploaded lately! This is a
big milestone towards the goal set out in http://lists.llvm.org/pipermail/cfe-dev/2019-August/063070.html.
I hope to accompany this with a patch where the a coreModeling package is added,
from which package diagnostics aren't allowed either, is an implicit dependency
of all checkers, and the core package for the first time can be safely disabled.

Differential Revision: https://reviews.llvm.org/D78126

Added: 
    

Modified: 
    clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
    clang/lib/StaticAnalyzer/Core/BugReporter.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
index 51565524db1e..27bc0dda1f1c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -19,6 +19,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
@@ -135,7 +136,7 @@ class BugReport {
   SmallVector<FixItHint, 4> Fixits;
 
   BugReport(Kind kind, const BugType &bt, StringRef desc)
-      : K(kind), BT(bt), Description(desc) {}
+      : BugReport(kind, bt, "", desc) {}
 
   BugReport(Kind K, const BugType &BT, StringRef ShortDescription,
             StringRef Description)
@@ -369,16 +370,13 @@ class PathSensitiveBugReport : public BugReport {
 public:
   PathSensitiveBugReport(const BugType &bt, StringRef desc,
                          const ExplodedNode *errorNode)
-      : BugReport(Kind::PathSensitive, bt, desc), ErrorNode(errorNode),
-        ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
-                                 : SourceRange()) {}
+      : PathSensitiveBugReport(bt, desc, desc, errorNode) {}
 
   PathSensitiveBugReport(const BugType &bt, StringRef shortDesc, StringRef desc,
                          const ExplodedNode *errorNode)
-      : BugReport(Kind::PathSensitive, bt, shortDesc, desc),
-        ErrorNode(errorNode),
-        ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
-                                 : SourceRange()) {}
+      : PathSensitiveBugReport(bt, shortDesc, desc, errorNode,
+                               /*LocationToUnique*/ {},
+                               /*DeclToUnique*/ nullptr) {}
 
   /// Create a PathSensitiveBugReport with a custom uniqueing location.
   ///
@@ -391,11 +389,13 @@ class PathSensitiveBugReport : public BugReport {
                          const ExplodedNode *errorNode,
                          PathDiagnosticLocation LocationToUnique,
                          const Decl *DeclToUnique)
-      : BugReport(Kind::PathSensitive, bt, desc), ErrorNode(errorNode),
-        ErrorNodeRange(getStmt() ? getStmt()->getSourceRange() : SourceRange()),
-        UniqueingLocation(LocationToUnique), UniqueingDecl(DeclToUnique) {
-    assert(errorNode);
-  }
+      : PathSensitiveBugReport(bt, desc, desc, errorNode, LocationToUnique,
+                               DeclToUnique) {}
+
+  PathSensitiveBugReport(const BugType &bt, StringRef shortDesc, StringRef desc,
+                         const ExplodedNode *errorNode,
+                         PathDiagnosticLocation LocationToUnique,
+                         const Decl *DeclToUnique);
 
   static bool classof(const BugReport *R) {
     return R->getKind() == Kind::PathSensitive;

diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index f4fd495956aa..a832b90ddf0e 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -34,6 +34,7 @@
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/CheckerRegistryData.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h"
@@ -2107,6 +2108,32 @@ void BuiltinBug::anchor() {}
 // Methods for BugReport and subclasses.
 //===----------------------------------------------------------------------===//
 
+static bool isDependency(const CheckerRegistryData &Registry,
+                         StringRef CheckerName) {
+  for (const std::pair<StringRef, StringRef> &Pair : Registry.Dependencies) {
+    if (Pair.second == CheckerName)
+      return true;
+  }
+  return false;
+}
+
+PathSensitiveBugReport::PathSensitiveBugReport(
+    const BugType &bt, StringRef shortDesc, StringRef desc,
+    const ExplodedNode *errorNode, PathDiagnosticLocation LocationToUnique,
+    const Decl *DeclToUnique)
+    : BugReport(Kind::PathSensitive, bt, shortDesc, desc), ErrorNode(errorNode),
+      ErrorNodeRange(getStmt() ? getStmt()->getSourceRange() : SourceRange()),
+      UniqueingLocation(LocationToUnique), UniqueingDecl(DeclToUnique) {
+  assert(!isDependency(ErrorNode->getState()
+                           ->getAnalysisManager()
+                           .getCheckerManager()
+                           ->getCheckerRegistryData(),
+                       bt.getCheckerName()) &&
+         "Some checkers depend on this one! We don't allow dependency "
+         "checkers to emit warnings, because checkers should depend on "
+         "*modeling*, not *diagnostics*.");
+}
+
 void PathSensitiveBugReport::addVisitor(
     std::unique_ptr<BugReporterVisitor> visitor) {
   if (!visitor)
@@ -2195,12 +2222,12 @@ static void insertToInterestingnessMap(
       return;
     case bugreporter::TrackingKind::Condition:
       return;
-  }
+    }
 
-  llvm_unreachable(
-      "BugReport::markInteresting currently can only handle 2 
diff erent "
-      "tracking kinds! Please define what tracking kind should this entitiy"
-      "have, if it was already marked as interesting with a 
diff erent kind!");
+    llvm_unreachable(
+        "BugReport::markInteresting currently can only handle 2 
diff erent "
+        "tracking kinds! Please define what tracking kind should this entitiy"
+        "have, if it was already marked as interesting with a 
diff erent kind!");
 }
 
 void PathSensitiveBugReport::markInteresting(SymbolRef sym,