sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay,
ioeric, ilya-biryukov, mgorny, srhines.
This runs checks over a restricted subset of the TU:
- preprocessor callbacks just receive the truncated PP events that occur when a
preamble is used.
- ASTMatchers run only over the top-level decls in the main-file
This patch just turns on one simple check (bugprone-sizeof-expression)
with no configuration.
- configuration is complex enough to warrant a separate patch
- arbitrary checks don't work well yet - there are various ways that checks can
access the whole AST (and thus be incredibly slow). Most notably: the
hasAncestor matcher, and using the ASTContext from check().
This depends on a small patch to ASTMatchers to run a MatchFinder on a
certain set of top-level declarations.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54204
Files:
clangd/CMakeLists.txt
clangd/ClangdUnit.cpp
unittests/clangd/ClangdUnitTests.cpp
Index: unittests/clangd/ClangdUnitTests.cpp
===================================================================
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -128,6 +128,15 @@
WithFix(Fix(Test.range(), "int", "change return type to 'int'")))));
}
+TEST(DiagnosticsTest, ClangTidy) {
+ Annotations Test("int main() { return [[sizeof]](sizeof(int)); }");
+ auto TU = TestTU::withCode(Test.code());
+ EXPECT_THAT(TU.build().getDiagnostics(),
+ ElementsAre(Diag(Test.range(),
+ "suspicious usage of 'sizeof(sizeof(...))' "
+ "[bugprone-sizeof-expression]")));
+}
+
TEST(DiagnosticsTest, Preprocessor) {
// This looks like a preamble, but there's an #else in the middle!
// Check that:
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -8,6 +8,8 @@
//===----------------------------------------------------------------------===//
#include "ClangdUnit.h"
+#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
+#include "../clang-tidy/ClangTidyModuleRegistry.h"
#include "Compiler.h"
#include "Diagnostics.h"
#include "Logger.h"
@@ -148,6 +150,34 @@
return None;
}
+ // Set up ClangTidy. Must happen after BeginSourceFile() so ASTContext exists.
+ std::vector<std::unique_ptr<tidy::ClangTidyCheck>> CTChecks;
+ ast_matchers::MatchFinder CTFinder;
+ llvm::Optional<tidy::ClangTidyContext> CTContext;
+ {
+ trace::Span Tracer("ClangTidyInit");
+ tidy::ClangTidyCheckFactories CTFactories;
+ for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
+ E.instantiate()->addCheckFactories(CTFactories);
+ auto CTOpts = tidy::ClangTidyOptions::getDefaults();
+ // FIXME: this needs to be configurable, and we need to support .clang-tidy
+ // files and other options providers.
+ // There is an important bug to fix before turning on arbitrary checks:
+ // we must restrict the AST parent map to the current file for performance.
+ // The placeholder check here does not use hasAncestor() so is unaffected.
+ CTOpts.Checks = "bugprone-sizeof-expression";
+ CTContext.emplace(llvm::make_unique<tidy::DefaultOptionsProvider>(
+ tidy::ClangTidyGlobalOptions(), CTOpts));
+ CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
+ CTContext->setASTContext(&Clang->getASTContext());
+ CTContext->setCurrentFile(MainInput.getFile());
+ CTFactories.createChecks(CTContext.getPointer(), CTChecks);
+ for (const auto &Check : CTChecks) {
+ Check->registerPPCallbacks(*Clang);
+ Check->registerMatchers(&CTFinder);
+ }
+ }
+
// Copy over the includes from the preamble, then combine with the
// non-preamble includes below.
auto Includes = Preamble ? Preamble->Includes : IncludeStructure{};
@@ -157,13 +187,24 @@
if (!Action->Execute())
log("Execute() failed when building AST for {0}", MainInput.getFile());
+ std::vector<Decl *> ParsedDecls = Action->takeTopLevelDecls();
+ {
+ // Run the AST-dependent part of the clang-tidy checks.
+ // (The preprocessor part ran already, via PPCallbacks).
+ trace::Span Tracer("ClangTidyMatch");
+ CTFinder.matchAST(Clang->getASTContext(), ParsedDecls);
+ }
+
// UnitDiagsConsumer is local, we can not store it in CompilerInstance that
// has a longer lifetime.
Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
// CompilerInstance won't run this callback, do it directly.
ASTDiags.EndSourceFile();
+ // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
+ // However Action->EndSourceFile() would destroy the ASTContext!
+ // So just inform the preprocessor of EOF, while keeping everything alive.
+ Clang->getPreprocessor().EndSourceFile();
- std::vector<Decl *> ParsedDecls = Action->takeTopLevelDecls();
std::vector<Diag> Diags = ASTDiags.take();
// Add diagnostics from the preamble, if any.
if (Preamble)
@@ -179,7 +220,12 @@
ParsedAST::~ParsedAST() {
if (Action) {
- Action->EndSourceFile();
+ // We already notified the PP of end-of-file earlier, so detach it first.
+ // We must keep it alive until after EndSourceFile(), Sema relies on this.
+ auto PP = Clang->getPreprocessorPtr(); // Keep PP alive for now.
+ Clang->setPreprocessor(nullptr); // Detach so we don't send EOF again.
+ Action->EndSourceFile(); // Destroy ASTContext and Sema.
+ // Now Sema is gone, it's safe for PP to go out of scope.
}
}
@@ -413,4 +459,29 @@
}
} // namespace clangd
+namespace tidy {
+// Force the linker to link in Clang-tidy modules.
+#define LINK_TIDY_MODULE(X) \
+ extern volatile int X##ModuleAnchorSource; \
+ static int LLVM_ATTRIBUTE_UNUSED X##ModuleAnchorDestination = \
+ X##ModuleAnchorSource
+LINK_TIDY_MODULE(CERT);
+LINK_TIDY_MODULE(Abseil);
+LINK_TIDY_MODULE(Boost);
+LINK_TIDY_MODULE(Bugprone);
+LINK_TIDY_MODULE(LLVM);
+LINK_TIDY_MODULE(CppCoreGuidelines);
+LINK_TIDY_MODULE(Fuchsia);
+LINK_TIDY_MODULE(Google);
+LINK_TIDY_MODULE(Android);
+LINK_TIDY_MODULE(Misc);
+LINK_TIDY_MODULE(Modernize);
+LINK_TIDY_MODULE(Performance);
+LINK_TIDY_MODULE(Portability);
+LINK_TIDY_MODULE(Readability);
+LINK_TIDY_MODULE(ObjC);
+LINK_TIDY_MODULE(HICPP);
+LINK_TIDY_MODULE(Zircon);
+#undef LINK_TIDY_MODULE
+} // namespace tidy
} // namespace clang
Index: clangd/CMakeLists.txt
===================================================================
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -64,6 +64,24 @@
clangLex
clangSema
clangSerialization
+ clangTidy
+ clangTidyAndroidModule
+ clangTidyAbseilModule
+ clangTidyBoostModule
+ clangTidyBugproneModule
+ clangTidyCERTModule
+ clangTidyCppCoreGuidelinesModule
+ clangTidyFuchsiaModule
+ clangTidyGoogleModule
+ clangTidyHICPPModule
+ clangTidyLLVMModule
+ clangTidyMiscModule
+ clangTidyModernizeModule
+ clangTidyObjCModule
+ clangTidyPerformanceModule
+ clangTidyPortabilityModule
+ clangTidyReadabilityModule
+ clangTidyZirconModule
clangTooling
clangToolingCore
clangToolingInclusions
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
