[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL347036: [clangd] Initial clang-tidy diagnostics support.
(authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D54204
Files:
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
Index: clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ClangdUnitTests.cpp
@@ -24,6 +24,7 @@
using testing::Field;
using testing::IsEmpty;
using testing::Pair;
+using testing::UnorderedElementsAre;
testing::Matcher WithFix(testing::Matcher FixMatcher) {
return Field(&Diag::Fixes, ElementsAre(FixMatcher));
@@ -128,6 +129,30 @@
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:
Index: clang-tools-extra/trunk/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/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
Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -672,8 +672,7 @@
return {};
DeducedTypeVisitor V(SourceLocationBeg);
- for (Decl *D : AST.getLocalTopLevelDecls())
-V.TraverseDecl(D);
+ V.TraverseAST(AST.getASTContext());
return V.getDeducedType();
}
Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/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 @@
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> CTChecks;
+ ast_matchers::MatchFinder CTFinder;
+ llvm::Optional 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-expre
[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. looks good! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D54204 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.
sammccall updated this revision to Diff 174204.
sammccall added a comment.
Remove clang-tidy changes, add FIXME comment.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54204
Files:
clangd/CMakeLists.txt
clangd/ClangdUnit.cpp
clangd/XRefs.cpp
unittests/clangd/ClangdUnitTests.cpp
Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -24,6 +24,7 @@
using testing::Field;
using testing::IsEmpty;
using testing::Pair;
+using testing::UnorderedElementsAre;
testing::Matcher WithFix(testing::Matcher FixMatcher) {
return Field(&Diag::Fixes, ElementsAre(FixMatcher));
@@ -128,6 +129,30 @@
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:
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -672,8 +672,7 @@
return {};
DeducedTypeVisitor V(SourceLocationBeg);
- for (Decl *D : AST.getLocalTopLevelDecls())
-V.TraverseDecl(D);
+ V.TraverseAST(AST.getASTContext());
return V.getDeducedType();
}
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"
@@ -151,6 +153,40 @@
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> CTChecks;
+ ast_matchers::MatchFinder CTFinder;
+ llvm::Optional 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::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 @@
if (!Action->Execute())
log("Execute() failed when building AST for {0}", MainInput.getFile());
+ std::vector 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 che
[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.
sammccall added a comment.
Moved the clang-tidy changes to https://reviews.llvm.org/D54579.
Sorry for mixing everything up!
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:518
void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(translationUnitDecl().bind("top"), this);
+ Finder->addMatcher(matchOnce(&MatchedOnce), this);
hokein wrote:
> maybe add a comment describing we are trying to find the top level decl?
We're not, we're trying to match any node (but only one). Extended the comment
on the matcher.
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:566
replaceCompoundReturnWithCondition(Result, Compound, true);
- else if (const auto TU = Result.Nodes.getNodeAs("top"))
-Visitor(this, Result).TraverseDecl(const_cast(TU));
+ else // MatchOnce matcher
+Visitor(this, Result).TraverseAST(*Result.Context);
hokein wrote:
> add an `assert (MatchOnce)`?
That doesn't compile, I'm not sure what you want here.
Comment at: clangd/ClangdUnit.cpp:158
+ // 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
hokein wrote:
> just want to make sure -- do we receive all preprocessor events in the main
> file when we use preamble? We have a few checks that generate `#include`
> insertion as part of FixIt.
>
> `TestTU` doesn't use preamble, it would be nice to have a test running on a
> main file with a real preamble, but this can be a follow-up I think.
Nope, you're right: `#include` from the preamble are not replayed. Can't fix
this now, added a FIXME. (Neither of the hardcoded checks care).
TestTU does use a preamble now :-)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54204
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.
hokein added a comment.
Looks mostly good, just a few nits.
This patch contains two parts (clang-tidy and clangd), I think we could split
into two, but I'm not insisting, up to you.
Comment at: clang-tidy/modernize/LoopConvertUtils.h:59
/// \brief Run the analysis on the TranslationUnitDecl.
///
The comment is out-of-date.
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:518
void SimplifyBooleanExprCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(translationUnitDecl().bind("top"), this);
+ Finder->addMatcher(matchOnce(&MatchedOnce), this);
maybe add a comment describing we are trying to find the top level decl?
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:566
replaceCompoundReturnWithCondition(Result, Compound, true);
- else if (const auto TU = Result.Nodes.getNodeAs("top"))
-Visitor(this, Result).TraverseDecl(const_cast(TU));
+ else // MatchOnce matcher
+Visitor(this, Result).TraverseAST(*Result.Context);
add an `assert (MatchOnce)`?
Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:82
+ bool MatchedOnce = false;
const bool ChainedConditionalReturn;
The name seems a bit unclear for readers without any context, maybe
`FoundTopDecl`?
Comment at: clangd/ClangdUnit.cpp:158
+ // 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
just want to make sure -- do we receive all preprocessor events in the main
file when we use preamble? We have a few checks that generate `#include`
insertion as part of FixIt.
`TestTU` doesn't use preamble, it would be nice to have a test running on a
main file with a real preamble, but this can be a follow-up I think.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54204
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.
sammccall added inline comments.
Comment at: clangd/ClangdUnit.cpp:175
+CTFactories.createChecks(CTContext.getPointer(), CTChecks);
+for (const auto &Check : CTChecks) {
+ Check->registerPPCallbacks(*Clang);
hokein wrote:
> Maybe add the check names to the `Trace`?
that would be nice, but there's no API to get that info :-(
Comment at: clangd/ClangdUnit.cpp:468
+ X##ModuleAnchorSource
+LINK_TIDY_MODULE(CERT);
+LINK_TIDY_MODULE(Abseil);
hokein wrote:
> I'm curious how much does clangd binary size get increased.
It's now 21M stripped vs 18M before this patch.
The different with debug info is *much* larger for some reason... sadly this
will hurt link times.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54204
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.
sammccall updated this revision to Diff 173357.
sammccall marked an inline comment as done.
sammccall added a comment.
Address comments and rebase on https://reviews.llvm.org/D54309, addressing
performance issues.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54204
Files:
clang-tidy/misc/UnusedParametersCheck.cpp
clang-tidy/modernize/LoopConvertCheck.cpp
clang-tidy/modernize/LoopConvertUtils.h
clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tidy/readability/SimplifyBooleanExprCheck.h
clangd/CMakeLists.txt
clangd/ClangdUnit.cpp
clangd/XRefs.cpp
unittests/clangd/ClangdUnitTests.cpp
Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -24,6 +24,7 @@
using testing::Field;
using testing::IsEmpty;
using testing::Pair;
+using testing::UnorderedElementsAre;
testing::Matcher WithFix(testing::Matcher FixMatcher) {
return Field(&Diag::Fixes, ElementsAre(FixMatcher));
@@ -128,6 +129,30 @@
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:
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -672,8 +672,7 @@
return {};
DeducedTypeVisitor V(SourceLocationBeg);
- for (Decl *D : AST.getLocalTopLevelDecls())
-V.TraverseDecl(D);
+ V.TraverseAST(AST.getASTContext());
return V.getDeducedType();
}
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"
@@ -151,6 +153,38 @@
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> CTChecks;
+ ast_matchers::MatchFinder CTFinder;
+ llvm::Optional 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::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{};
@@ -160,13 +194,26 @@
if (!Action->Execute())
log("Execute() failed when building AST for {0}", MainInput.getFile());
+ std::vector ParsedDecls = Action->takeTopLevelDecls();
+ //
[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.
hokein added a comment.
I think this is in a good shape as initial patch!
Comment at: clangd/ClangdUnit.cpp:168
+// The placeholder check here does not use hasAncestor() so is unaffected.
+CTOpts.Checks = "bugprone-sizeof-expression";
+CTContext.emplace(llvm::make_unique(
It seems we also support clang-tidy checks that analyze preprocessor-dependent
properties. I think we can add a clang-tidy check to make sure `PPCallback`
actually work, `google-readability-todo` is a good candidate.
Ah just realized limitations (truncated `PPCallback` events) you wrote in the
patch description, maybe mention them in the source comment, so that we won't
forget in the future when reading the code.
Comment at: clangd/ClangdUnit.cpp:175
+CTFactories.createChecks(CTContext.getPointer(), CTChecks);
+for (const auto &Check : CTChecks) {
+ Check->registerPPCallbacks(*Clang);
Maybe add the check names to the `Trace`?
Comment at: clangd/ClangdUnit.cpp:468
+ X##ModuleAnchorSource
+LINK_TIDY_MODULE(CERT);
+LINK_TIDY_MODULE(Abseil);
I'm curious how much does clangd binary size get increased.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54204
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.
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> CTChecks;
+ ast_matchers::MatchFinder CTFinder;
+ llvm::Optional 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::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 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 ParsedDecls = Action->takeTopLevelDecls();
std::vector Diags = ASTDiags.take();
// Add diagnostics from the preamble, if any.
if (Pream
