Revert "[analyzer][NFC] Don't allow dependency checkers to emit diagnostics"

Summary:
This reverts commit 33fb9cbe21.

That commit violates layering by adding a dependency from StaticAnalyzer/Core
back to StaticAnalyzer/FrontEnd, creating a circular dependency.

I can't see a clean way to fix it except refactoring.

Reviewers: echristo, Szelethus, martong

Subscribers: xazax.hun, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, ASDenysPetrov, cfe-commits

Tags: #clang

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

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

@@ -19,7 +19,6 @@
 #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"
@@ -136,7 +135,7 @@ protected:
   SmallVector<FixItHint, 4> Fixits;
 
   BugReport(Kind kind, const BugType &bt, StringRef desc)
-      : BugReport(kind, bt, "", desc) {}
+      : K(kind), BT(bt), Description(desc) {}
 
   BugReport(Kind K, const BugType &BT, StringRef ShortDescription,
             StringRef Description)
@@ -370,13 +369,16 @@ protected:
 public:
   PathSensitiveBugReport(const BugType &bt, StringRef desc,
                          const ExplodedNode *errorNode)
-      : PathSensitiveBugReport(bt, desc, desc, errorNode) {}
+      : BugReport(Kind::PathSensitive, bt, desc), ErrorNode(errorNode),
+        ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+                                 : SourceRange()) {}
 
   PathSensitiveBugReport(const BugType &bt, StringRef shortDesc, StringRef desc,
                          const ExplodedNode *errorNode)
-      : PathSensitiveBugReport(bt, shortDesc, desc, errorNode,
-                               /*LocationToUnique*/ {},
-                               /*DeclToUnique*/ nullptr) {}
+      : BugReport(Kind::PathSensitive, bt, shortDesc, desc),
+        ErrorNode(errorNode),
+        ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+                                 : SourceRange()) {}
 
   /// Create a PathSensitiveBugReport with a custom uniqueing location.
   ///
@@ -389,13 +391,11 @@ public:
                          const ExplodedNode *errorNode,
                          PathDiagnosticLocation LocationToUnique,
                          const Decl *DeclToUnique)
-      : PathSensitiveBugReport(bt, desc, desc, errorNode, LocationToUnique,
-                               DeclToUnique) {}
-
-  PathSensitiveBugReport(const BugType &bt, StringRef shortDesc, StringRef desc,
-                         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);
+  }
 
   static bool classof(const BugReport *R) {
     return R->getKind() == Kind::PathSensitive;

clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h

@@ -135,7 +135,7 @@ public:
              "Invalid development status!");
     }
 
-    LLVM_DUMP_METHOD void dump() const;
+    LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
     LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream &Out) const;
   };
 
@@ -195,7 +195,7 @@ public:
     // Used for lower_bound.
     explicit CheckerInfo(StringRef FullName) : FullName(FullName) {}
 
-    LLVM_DUMP_METHOD void dump() const;
+    LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
     LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream &Out) const;
   };
 
@@ -215,7 +215,7 @@ public:
 
     explicit PackageInfo(StringRef FullName) : FullName(FullName) {}
 
-    LLVM_DUMP_METHOD void dump() const;
+    LLVM_DUMP_METHOD void dump() const { dumpToStream(llvm::errs()); }
     LLVM_DUMP_METHOD void dumpToStream(llvm::raw_ostream &Out) const;
   };
 
@@ -317,30 +317,30 @@ private:
   /// For example, it'll return the checkers for the core package, if
   /// \p CmdLineArg is "core".
   CheckerInfoListRange getMutableCheckersForCmdLineArg(StringRef CmdLineArg);
+
+  CheckerInfoList Checkers;
+  PackageInfoList Packages;
   /// Used for couting how many checkers belong to a certain package in the
   /// \c Checkers field. For convenience purposes.
   llvm::StringMap<size_t> PackageSizes;
 
-  void resolveCheckerAndPackageOptions();
-  template <bool IsWeak> void resolveDependencies();
-
-  DiagnosticsEngine &Diags;
-  AnalyzerOptions &AnOpts;
-
-public:
-  CheckerInfoList Checkers;
-  PackageInfoList Packages;
-
   /// Contains all (Dependendent checker, Dependency) pairs. We need this, as
   /// we'll resolve dependencies after all checkers were added first.
   llvm::SmallVector<std::pair<StringRef, StringRef>, 0> Dependencies;
   llvm::SmallVector<std::pair<StringRef, StringRef>, 0> WeakDependencies;
 
+  template <bool IsWeak> void resolveDependencies();
+
   /// Contains all (FullName, CmdLineOption) pairs. Similarly to dependencies,
   /// we only modify the actual CheckerInfo and PackageInfo objects once all
   /// of them have been added.
   llvm::SmallVector<std::pair<StringRef, CmdLineOption>, 0> PackageOptions;
   llvm::SmallVector<std::pair<StringRef, CmdLineOption>, 0> CheckerOptions;
+
+  void resolveCheckerAndPackageOptions();
+
+  DiagnosticsEngine &Diags;
+  AnalyzerOptions &AnOpts;
   CheckerInfoSet EnabledCheckers;
 };
 

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

@@ -40,7 +40,6 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SVals.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
-#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -2107,32 +2106,6 @@ void BuiltinBug::anchor() {}
 // Methods for BugReport and subclasses.
 //===----------------------------------------------------------------------===//
 
-static bool isDependency(const CheckerRegistry &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()
-                           ->getCheckerRegistry(),
-                       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)
@@ -2221,12 +2194,12 @@ static void insertToInterestingnessMap(
       return;
     case bugreporter::TrackingKind::Condition:
       return;
-    }
+  }
 
-    llvm_unreachable(
-        "BugReport::markInteresting currently can only handle 2 different "
-        "tracking kinds! Please define what tracking kind should this entitiy"
-        "have, if it was already marked as interesting with a different kind!");
+  llvm_unreachable(
+      "BugReport::markInteresting currently can only handle 2 different "
+      "tracking kinds! Please define what tracking kind should this entitiy"
+      "have, if it was already marked as interesting with a different kind!");
 }
 
 void PathSensitiveBugReport::markInteresting(SymbolRef sym,

clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

@@ -94,10 +94,6 @@ static bool isInPackage(const CheckerRegistry::CheckerInfo &Checker,
 // Methods of CmdLineOption, PackageInfo and CheckerInfo.
 //===----------------------------------------------------------------------===//
 
-LLVM_DUMP_METHOD void CheckerRegistry::CmdLineOption::dump() const {
-  dumpToStream(llvm::errs());
-}
-
 LLVM_DUMP_METHOD void
 CheckerRegistry::CmdLineOption::dumpToStream(llvm::raw_ostream &Out) const {
   // The description can be just checked in Checkers.inc, the point here is to
@@ -119,10 +115,6 @@ static StringRef toString(CheckerRegistry::StateFromCmdLine Kind) {
   llvm_unreachable("Unhandled CheckerRegistry::StateFromCmdLine enum");
 }
 
-LLVM_DUMP_METHOD void CheckerRegistry::CheckerInfo::dump() const {
-  dumpToStream(llvm::errs());
-}
-
 LLVM_DUMP_METHOD void
 CheckerRegistry::CheckerInfo::dumpToStream(llvm::raw_ostream &Out) const {
   // The description can be just checked in Checkers.inc, the point here is to
@@ -145,10 +137,6 @@ CheckerRegistry::CheckerInfo::dumpToStream(llvm::raw_ostream &Out) const {
   }
 }
 
-LLVM_DUMP_METHOD void CheckerRegistry::PackageInfo::dump() const {
-  dumpToStream(llvm::errs());
-}
-
 LLVM_DUMP_METHOD void
 CheckerRegistry::PackageInfo::dumpToStream(llvm::raw_ostream &Out) const {
   Out << FullName << "\n";