Szelethus created this revision. Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, rnkovacs. Szelethus added a project: clang. Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity. Szelethus added a parent revision: D59459: [analyzer][NFC] Prefer binary searches in CheckerRegistry.
Ideally, there is no reason behind not being able to depend on checkers that come from a different plugin (or on builtin checkers) -- however, this is only possible if all checkers are added to the registry before resolving checker dependencies. Since I used a binary search in my `addDependency` method, this also resulted in an assertion failure (due to `CheckerRegistry::Checkers` not being sorted), since the function used by plugins to register their checkers (`clang_registerCheckers`) calls `addDependency`. This patch resolves this issue by only noting which dependencies have to established when `addDependency` is called, and resolves them at a later stage when no more checkers are added to the registry. Repository: rC Clang https://reviews.llvm.org/D59461 Files: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp test/Analysis/checker-dependencies.c Index: test/Analysis/checker-dependencies.c =================================================================== --- test/Analysis/checker-dependencies.c +++ test/Analysis/checker-dependencies.c @@ -1,3 +1,20 @@ // RUN: %clang_analyze_cc1 %s \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=nullability.NullReturnedFromNonnull + +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=osx.cocoa.RetainCount \ +// RUN: -analyzer-list-enabled-checkers \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-ENABLED + +// CHECK-IMPLICITLY-ENABLED: osx.cocoa.RetainCountBase +// CHECK-IMPLICITLY-ENABLED: osx.cocoa.RetainCount + +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=osx.cocoa.RetainCount \ +// RUN: -analyzer-disable-checker=osx.cocoa.RetainCountBase \ +// RUN: -analyzer-list-enabled-checkers \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-DISABLED + +// CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCountBase +// CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCount Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp =================================================================== --- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp @@ -180,6 +180,8 @@ #undef CHECKER_DEPENDENCY #undef GET_CHECKER_DEPENDENCIES + resolveDependencies(); + // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the // command line. for (const std::pair<std::string, bool> &Opt : AnOpts.CheckersControlList) { @@ -281,17 +283,25 @@ } } -void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) { - auto CheckerIt = binaryFind(Checkers, FullName); - assert(CheckerIt != Checkers.end() && - "Failed to find the checker while attempting to set up its " - "dependencies!"); +void CheckerRegistry::resolveDependencies() { + for (const std::pair<StringRef, StringRef> &Entry : Dependencies) { + auto CheckerIt = binaryFind(Checkers, Entry.first); + assert(CheckerIt != Checkers.end() && + "Failed to find the checker while attempting to set up its " + "dependencies!"); - auto DependencyIt = binaryFind(Checkers, Dependency); - assert(DependencyIt != Checkers.end() && - "Failed to find the dependency of a checker!"); + auto DependencyIt = binaryFind(Checkers, Entry.second); + assert(DependencyIt != Checkers.end() && + "Failed to find the dependency of a checker!"); + + CheckerIt->Dependencies.emplace_back(&*DependencyIt); + } - CheckerIt->Dependencies.emplace_back(&*DependencyIt); + Dependencies.clear(); +} + +void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) { + Dependencies.emplace_back(FullName, Dependency); } void CheckerRegistry::initializeManager(CheckerManager &CheckerMgr) const { Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h =================================================================== --- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h +++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h @@ -195,6 +195,12 @@ CheckerInfoList Checkers; 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; + + void resolveDependencies(); + DiagnosticsEngine &Diags; AnalyzerOptions &AnOpts; const LangOptions &LangOpts;
Index: test/Analysis/checker-dependencies.c =================================================================== --- test/Analysis/checker-dependencies.c +++ test/Analysis/checker-dependencies.c @@ -1,3 +1,20 @@ // RUN: %clang_analyze_cc1 %s \ // RUN: -analyzer-checker=core \ // RUN: -analyzer-checker=nullability.NullReturnedFromNonnull + +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=osx.cocoa.RetainCount \ +// RUN: -analyzer-list-enabled-checkers \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-ENABLED + +// CHECK-IMPLICITLY-ENABLED: osx.cocoa.RetainCountBase +// CHECK-IMPLICITLY-ENABLED: osx.cocoa.RetainCount + +// RUN: %clang_analyze_cc1 %s \ +// RUN: -analyzer-checker=osx.cocoa.RetainCount \ +// RUN: -analyzer-disable-checker=osx.cocoa.RetainCountBase \ +// RUN: -analyzer-list-enabled-checkers \ +// RUN: 2>&1 | FileCheck %s -check-prefix=CHECK-IMPLICITLY-DISABLED + +// CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCountBase +// CHECK-IMPLICITLY-DISABLED-NOT: osx.cocoa.RetainCount Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp =================================================================== --- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp +++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp @@ -180,6 +180,8 @@ #undef CHECKER_DEPENDENCY #undef GET_CHECKER_DEPENDENCIES + resolveDependencies(); + // Parse '-analyzer-checker' and '-analyzer-disable-checker' options from the // command line. for (const std::pair<std::string, bool> &Opt : AnOpts.CheckersControlList) { @@ -281,17 +283,25 @@ } } -void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) { - auto CheckerIt = binaryFind(Checkers, FullName); - assert(CheckerIt != Checkers.end() && - "Failed to find the checker while attempting to set up its " - "dependencies!"); +void CheckerRegistry::resolveDependencies() { + for (const std::pair<StringRef, StringRef> &Entry : Dependencies) { + auto CheckerIt = binaryFind(Checkers, Entry.first); + assert(CheckerIt != Checkers.end() && + "Failed to find the checker while attempting to set up its " + "dependencies!"); - auto DependencyIt = binaryFind(Checkers, Dependency); - assert(DependencyIt != Checkers.end() && - "Failed to find the dependency of a checker!"); + auto DependencyIt = binaryFind(Checkers, Entry.second); + assert(DependencyIt != Checkers.end() && + "Failed to find the dependency of a checker!"); + + CheckerIt->Dependencies.emplace_back(&*DependencyIt); + } - CheckerIt->Dependencies.emplace_back(&*DependencyIt); + Dependencies.clear(); +} + +void CheckerRegistry::addDependency(StringRef FullName, StringRef Dependency) { + Dependencies.emplace_back(FullName, Dependency); } void CheckerRegistry::initializeManager(CheckerManager &CheckerMgr) const { Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h =================================================================== --- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h +++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h @@ -195,6 +195,12 @@ CheckerInfoList Checkers; 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; + + void resolveDependencies(); + DiagnosticsEngine &Diags; AnalyzerOptions &AnOpts; const LangOptions &LangOpts;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits