[clang-tools-extra] r347036 - [clangd] Initial clang-tidy diagnostics support.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 16 00:32:23 PST 2018
Author: sammccall
Date: Fri Nov 16 00:32:23 2018
New Revision: 347036
URL: http://llvm.org/viewvc/llvm-project?rev=347036&view=rev
Log:
[clangd] Initial clang-tidy diagnostics support.
Summary:
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
This depends on a patch allowing traversal to be restricted to a scope.
Reviewers: hokein
Subscribers: srhines, mgorny, ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Differential Revision: https://reviews.llvm.org/D54204
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=347036&r1=347035&r2=347036&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Fri Nov 16 00:32:23 2018
@@ -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
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=347036&r1=347035&r2=347036&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Nov 16 00:32:23 2018
@@ -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<Compile
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<Compile
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=(ParsedAS
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(
}
} // 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
Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=347036&r1=347035&r2=347036&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Fri Nov 16 00:32:23 2018
@@ -672,8 +672,7 @@ Optional<QualType> getDeducedType(Parsed
return {};
DeducedTypeVisitor V(SourceLocationBeg);
- for (Decl *D : AST.getLocalTopLevelDecls())
- V.TraverseDecl(D);
+ V.TraverseAST(AST.getASTContext());
return V.getDeducedType();
}
Modified: clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp?rev=347036&r1=347035&r2=347036&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp Fri Nov 16 00:32:23 2018
@@ -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:
More information about the cfe-commits
mailing list