[clang] 33fb9cb - [analyzer][NFC] Don't allow dependency checkers to emit diagnostics
Kirstóf Umann via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 12 06:02:49 PDT 2020
Author: Kirstóf Umann
Date: 2020-06-12T14:59:48+02:00
New Revision: 33fb9cbe211d1b43d4b84edf34e11001f04cddf0
URL: https://github.com/llvm/llvm-project/commit/33fb9cbe211d1b43d4b84edf34e11001f04cddf0
DIFF: https://github.com/llvm/llvm-project/commit/33fb9cbe211d1b43d4b84edf34e11001f04cddf0.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/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 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/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
index 7b72fae0fefe..795067cba582 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 { dumpToStream(llvm::errs()); }
+ LLVM_DUMP_METHOD void dump() const;
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 { dumpToStream(llvm::errs()); }
+ LLVM_DUMP_METHOD void dump() const;
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 { dumpToStream(llvm::errs()); }
+ LLVM_DUMP_METHOD void dump() const;
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);
-
- 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;
};
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index efae511da83b..17f7036eff3d 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -40,6 +40,7 @@
#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"
@@ -2106,6 +2107,32 @@ 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)
@@ -2194,12 +2221,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 ce5e0497435c..c2ca9c12b025 100644
--- a/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -94,6 +94,10 @@ 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
@@ -115,6 +119,10 @@ 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
@@ -137,6 +145,10 @@ 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