diff options
| -rw-r--r-- | clang-tools-extra/clangd/CMakeLists.txt | 18 | ||||
| -rw-r--r-- | clang-tools-extra/clangd/ClangdUnit.cpp | 83 | ||||
| -rw-r--r-- | clang-tools-extra/clangd/XRefs.cpp | 3 | ||||
| -rw-r--r-- | clang-tools-extra/unittests/clangd/ClangdUnitTests.cpp | 25 |
4 files changed, 125 insertions, 4 deletions
diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt index ca68e3a9942..b3f4fc61831 100644 --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -64,6 +64,24 @@ add_clang_library(clangDaemon clangLex clangSema clangSerialization + clangTidy + clangTidyAndroidModule + clangTidyAbseilModule + clangTidyBoostModule + clangTidyBugproneModule + clangTidyCERTModule + clangTidyCppCoreGuidelinesModule + clangTidyFuchsiaModule + clangTidyGoogleModule + clangTidyHICPPModule + clangTidyLLVMModule + clangTidyMiscModule + clangTidyModernizeModule + clangTidyObjCModule + clangTidyPerformanceModule + clangTidyPortabilityModule + clangTidyReadabilityModule + clangTidyZirconModule clangTooling clangToolingCore clangToolingInclusions diff --git a/clang-tools-extra/clangd/ClangdUnit.cpp b/clang-tools-extra/clangd/ClangdUnit.cpp index eadd891826b..0d43c0b7a09 100644 --- a/clang-tools-extra/clangd/ClangdUnit.cpp +++ b/clang-tools-extra/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" @@ -151,6 +153,40 @@ ParsedAST::build(std::unique_ptr<CompilerInvocation> CI, return None; } + // Set up ClangTidy. Must happen after BeginSourceFile() so ASTContext exists. + // Clang-tidy has some limitiations to ensure reasonable performance: + // - checks don't see all preprocessor events in the preamble + // - matchers run only over the main-file top-level decls (and can't see + // ancestors outside this scope). + // In practice almost all checks work well without modifications. + 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. + // These checks exercise the matcher- and preprocessor-based hooks. + CTOpts.Checks = + "bugprone-sizeof-expression,bugprone-macro-repeated-side-effects"; + 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) { + // FIXME: the PP callbacks skip the entire preamble. + // Checks that want to see #includes in the main file do not see them. + 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{}; @@ -160,13 +196,26 @@ ParsedAST::build(std::unique_ptr<CompilerInvocation> CI, if (!Action->Execute()) log("Execute() failed when building AST for {0}", MainInput.getFile()); + std::vector<Decl *> ParsedDecls = Action->takeTopLevelDecls(); + // AST traversals should exclude the preamble, to avoid performance cliffs. + Clang->getASTContext().setTraversalScope(ParsedDecls); + { + // 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()); + } + // 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) @@ -182,7 +231,12 @@ ParsedAST &ParsedAST::operator=(ParsedAST &&Other) = default; 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. } } @@ -416,4 +470,29 @@ SourceLocation getBeginningOfIdentifier(ParsedAST &Unit, const Position &Pos, } } // 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 diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp index d01a67cae8f..b0ed6143a11 100644 --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -672,8 +672,7 @@ Optional<QualType> getDeducedType(ParsedAST &AST, return {}; DeducedTypeVisitor V(SourceLocationBeg); - for (Decl *D : AST.getLocalTopLevelDecls()) - V.TraverseDecl(D); + V.TraverseAST(AST.getASTContext()); return V.getDeducedType(); } diff --git a/clang-tools-extra/unittests/clangd/ClangdUnitTests.cpp b/clang-tools-extra/unittests/clangd/ClangdUnitTests.cpp index d7961da2ac5..b59d416ad97 100644 --- a/clang-tools-extra/unittests/clangd/ClangdUnitTests.cpp +++ b/clang-tools-extra/unittests/clangd/ClangdUnitTests.cpp @@ -24,6 +24,7 @@ using testing::ElementsAre; using testing::Field; using testing::IsEmpty; using testing::Pair; +using testing::UnorderedElementsAre; testing::Matcher<const Diag &> WithFix(testing::Matcher<Fix> FixMatcher) { return Field(&Diag::Fixes, ElementsAre(FixMatcher)); @@ -128,6 +129,30 @@ TEST(DiagnosticsTest, FlagsMatter) { WithFix(Fix(Test.range(), "int", "change return type to 'int'"))))); } +TEST(DiagnosticsTest, ClangTidy) { + Annotations Test(R"cpp( + #define $macrodef[[SQUARE]](X) (X)*(X) + int main() { + return [[sizeof]](sizeof(int)); + int y = 4; + return SQUARE($macroarg[[++]]y); + } + )cpp"); + auto TU = TestTU::withCode(Test.code()); + EXPECT_THAT( + TU.build().getDiagnostics(), + UnorderedElementsAre( + Diag(Test.range(), "suspicious usage of 'sizeof(sizeof(...))' " + "[bugprone-sizeof-expression]"), + AllOf( + Diag(Test.range("macroarg"), + "side effects in the 1st macro argument 'X' are repeated in " + "macro expansion [bugprone-macro-repeated-side-effects]"), + WithNote(Diag(Test.range("macrodef"), + "macro 'SQUARE' defined here " + "[bugprone-macro-repeated-side-effects]"))))); +} + TEST(DiagnosticsTest, Preprocessor) { // This looks like a preamble, but there's an #else in the middle! // Check that: |
