sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a reviewer: njames93.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
This uses the fast-check allowlist added in the previous commit.
This is behind a config option to allow users/developers to enable checks
we haven't timed yet, and to allow the --check-tidy-time flag to work.
https://github.com/clangd/clangd/issues/1337
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D138505
Files:
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/TidyProvider.cpp
clang-tools-extra/clangd/TidyProvider.h
clang-tools-extra/clangd/tool/Check.cpp
Index: clang-tools-extra/clangd/tool/Check.cpp
===================================================================
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -30,6 +30,8 @@
#include "CodeComplete.h"
#include "CompileCommands.h"
#include "Config.h"
+#include "ConfigFragment.h"
+#include "ConfigProvider.h"
#include "Feature.h"
#include "GlobalCompilationDatabase.h"
#include "Hover.h"
@@ -82,15 +84,19 @@
"check-completion",
llvm::cl::desc("Run code-completion at each point (slow)"),
llvm::cl::init(false)};
+llvm::cl::opt<bool> CheckWarnings{
+ "check-warnings",
+ llvm::cl::desc("Print warnings as well as errors"),
+ llvm::cl::init(false)};
// Print (and count) the error-level diagnostics (warnings are ignored).
unsigned showErrors(llvm::ArrayRef<Diag> Diags) {
unsigned ErrCount = 0;
for (const auto &D : Diags) {
- if (D.Severity >= DiagnosticsEngine::Error) {
+ if (D.Severity >= DiagnosticsEngine::Error || CheckWarnings)
elog("[{0}] Line {1}: {2}", D.Name, D.Range.start.line + 1, D.Message);
+ if (D.Severity >= DiagnosticsEngine::Error)
++ErrCount;
- }
}
return ErrCount;
}
@@ -452,8 +458,23 @@
}
log("Testing on source file {0}", File);
+ class OverrideConfigProvider : public config::Provider {
+ std::vector<config::CompiledFragment>
+ getFragments(const config::Params &,
+ config::DiagnosticCallback Diag) const override {
+ config::Fragment F;
+ // If we're timing clang-tidy checks, implicitly disabling the slow ones
+ // is counterproductive!
+ if (CheckTidyTime.getNumOccurrences())
+ F.Diagnostics.ClangTidy.SlowChecks = true;
+ return {std::move(F).compile(Diag)};
+ }
+ } OverrideConfig;
+ auto ConfigProvider =
+ config::Provider::combine({Opts.ConfigProvider, &OverrideConfig});
+
auto ContextProvider = ClangdServer::createConfiguredContextProvider(
- Opts.ConfigProvider, nullptr);
+ ConfigProvider.get(), nullptr);
WithContext Ctx(ContextProvider(
FakeFile.empty()
? File
Index: clang-tools-extra/clangd/TidyProvider.h
===================================================================
--- clang-tools-extra/clangd/TidyProvider.h
+++ clang-tools-extra/clangd/TidyProvider.h
@@ -60,6 +60,10 @@
/// \pre \p must not be empty, must not contain '*' or ',' or start with '-'.
bool isRegisteredTidyCheck(llvm::StringRef Check);
+/// Returns if \p Check is a known-fast clang-tidy check.
+/// By default, only such checks will run in clangd.
+bool isFastTidyCheck(llvm::StringRef Check);
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/TidyProvider.cpp
===================================================================
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -213,13 +213,7 @@
// tries to build an AST.
"-bugprone-use-after-move",
// Alias for bugprone-use-after-move.
- "-hicpp-invalid-access-moved",
-
- // ----- Performance problems -----
-
- // This check runs expensive analysis for each variable.
- // It has been observed to increase reparse time by 10x.
- "-misc-const-correctness");
+ "-hicpp-invalid-access-moved");
size_t Size = BadChecks.size();
for (const std::string &Str : ExtraBadChecks) {
@@ -308,5 +302,14 @@
return AllChecks.contains(Check);
}
+
+bool isFastTidyCheck(llvm::StringRef Check) {
+ static auto &Fast = *new llvm::StringSet<>{
+#define FAST(CHECK, TIME) #CHECK,
+#include "TidyFastChecks.inc"
+ };
+ return Fast.contains(Check);
+}
+
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -345,6 +345,7 @@
std::shared_ptr<const PreambleData> Preamble) {
trace::Span Tracer("BuildAST");
SPAN_ATTACH(Tracer, "File", Filename);
+ const Config &Cfg = Config::current();
auto VFS = Inputs.TFS->view(Inputs.CompileCommand.Directory);
if (Preamble && Preamble->StatCache)
@@ -476,6 +477,15 @@
tidy::ClangTidyCheckFactories CTFactories;
for (const auto &E : tidy::ClangTidyModuleRegistry::entries())
E.instantiate()->addCheckFactories(CTFactories);
+ if (!Cfg.Diagnostics.ClangTidy.SlowChecks) {
+ // Can't remove checks from a factories container, so create a new one.
+ tidy::ClangTidyCheckFactories FastFactories;
+ for (const auto &Factory : CTFactories) {
+ if (isFastTidyCheck(Factory.getKey()))
+ FastFactories.registerCheckFactory(Factory.first(), Factory.second);
+ }
+ CTFactories = std::move(FastFactories);
+ }
CTContext.emplace(std::make_unique<tidy::DefaultOptionsProvider>(
tidy::ClangTidyGlobalOptions(), ClangTidyOpts));
CTContext->setDiagnosticsEngine(&Clang->getDiagnostics());
@@ -504,7 +514,6 @@
SourceLocation());
}
- const Config &Cfg = Config::current();
ASTDiags.setLevelAdjuster([&](DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) {
if (Cfg.Diagnostics.SuppressAll ||
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -152,6 +152,10 @@
});
CheckOptDict.parse(N);
});
+ Dict.handle("SlowChecks", [&](Node &N) {
+ if (auto SlowChecks = boolValue(N, "SlowChecks"))
+ F.SlowChecks = *SlowChecks;
+ });
Dict.parse(N);
}
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -264,6 +264,11 @@
/// readability-braces-around-statements.ShortStatementLines: 2
std::vector<std::pair<Located<std::string>, Located<std::string>>>
CheckOptions;
+
+ /// Whether to run checks that may slow down clangd.
+ /// By default, only known-fast checks are executed.
+ /// This may exclude recently-added fast checks we have not timed yet.
+ llvm::Optional<Located<bool>> SlowChecks;
};
ClangTidyBlock ClangTidy;
};
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -514,6 +514,11 @@
StringPair.first, StringPair.second);
});
}
+
+ if (F.SlowChecks.has_value())
+ Out.Apply.push_back([V = **F.SlowChecks](const Params &, Config &C) {
+ C.Diagnostics.ClangTidy.SlowChecks = V;
+ });
}
void compile(Fragment::DiagnosticsBlock::IncludesBlock &&F) {
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -99,6 +99,7 @@
// A comma-seperated list of globs specify which clang-tidy checks to run.
std::string Checks;
llvm::StringMap<std::string> CheckOptions;
+ bool SlowChecks = false;
} ClangTidy;
UnusedIncludesPolicy UnusedIncludes = None;
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
