[clang] e640598 - Revert "[analyzer][NFC] Don't allow dependency checkers to emit diagnostics"
Sterling Augustine via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 12 12:10:33 PDT 2020
Author: Sterling Augustine
Date: 2020-06-12T12:10:13-07:00
New Revision: e64059828f19f629081220bffeb3ef7582870111
URL: https://github.com/llvm/llvm-project/commit/e64059828f19f629081220bffeb3ef7582870111
DIFF: https://github.com/llvm/llvm-project/commit/e64059828f19f629081220bffeb3ef7582870111.diff
LOG: Revert "[analyzer][NFC] Don't allow dependency checkers to emit diagnostics"
Summary:
This reverts commit 33fb9cbe211d1b43d4b84edf34e11001f04cddf0.
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
Added:
Modified:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
index 27bc0dda1f1c..51565524db1e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ b/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 @@ class BugReport {
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 @@ class PathSensitiveBugReport : public BugReport {
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 @@ class PathSensitiveBugReport : public BugReport {
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;
diff --git a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
index 795067cba582..7b72fae0fefe 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -135,7 +135,7 @@ class CheckerRegistry {
"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 @@ class CheckerRegistry {
// 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 @@ class CheckerRegistry {
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 @@ class CheckerRegistry {
/// For example, it'll return the checkers for the core package, if
/// \p CmdLineArg is "core".
CheckerInfoListRange getMutableCheckersForCmdLineArg(StringRef CmdLineArg);
- /// 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;
+ /// Used for couting how many checkers belong to a certain package in the
+ /// \c Checkers field. For convenience purposes.
+ llvm::StringMap<size_t> PackageSizes;
/// 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;
};
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index 17f7036eff3d..efae511da83b 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/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
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,
diff --git a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
index 461d08f3d2c7..6d0e36355b4c 100644
--- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ b/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";
More information about the cfe-commits
mailing list