PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Currently due to missing DiagnosticsEngine when invalid
enum config value is passed to check, and --dump-config
is used, clang-tidy would crash. This change fixes this
and extends --verify-config command to actually verify
those invalid enum values instead of ignoring them and
printing that 'No config errors detected.'.
Fixes:
- https://github.com/llvm/llvm-project/issues/50876
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D146842
Files:
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/clang-tidy/ClangTidy.h
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/test/clang-tidy/infrastructure/invalid-enum-config-value.cpp
Index: clang-tools-extra/test/clang-tidy/infrastructure/invalid-enum-config-value.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/invalid-enum-config-value.cpp
@@ -0,0 +1,11 @@
+// RUN: clang-tidy --checks="-*,modernize-make-shared" \
+// RUN: --config="CheckOptions: [{ \
+// RUN: key: modernize-make-shared.IncludeStyle, value: '0' }]" \
+// RUN: --dump-config -- | FileCheck %s -check-prefix=CHECK-DUMP
+// RUN: not clang-tidy --checks="-*,modernize-make-shared" \
+// RUN: --config="CheckOptions: [{ \
+// RUN: key: modernize-make-shared.IncludeStyle, value: '0' }]" \
+// RUN: --verify-config -- 2>&1 | FileCheck %s -check-prefix=CHECK-VERIFY
+
+// CHECK-DUMP: modernize-make-shared.IncludeStyle: llvm
+// CHECK-VERIFY: command-line option '-config': warning: invalid configuration value '0' for option 'modernize-make-shared.IncludeStyle' [clang-tidy-config]
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -511,6 +511,63 @@
return AnyInvalid;
}
+static raw_ostream &getDiagnosticForLevel(tooling::Diagnostic::Level DiagLevel,
+ llvm::StringRef Prefix = "") {
+ switch (DiagLevel) {
+ case tooling::Diagnostic::Error:
+ return llvm::WithColor::error(llvm::errs(), Prefix);
+ case tooling::Diagnostic::Warning:
+ return llvm::WithColor::warning(llvm::errs(), Prefix);
+ case tooling::Diagnostic::Remark:
+ return llvm::WithColor::remark(llvm::errs(), Prefix);
+ }
+}
+
+static llvm::StringSet<> findSourcesForDiagnostic(
+ const std::vector<ClangTidyOptionsProvider::OptionsSource> &RawOptions,
+ llvm::StringRef Diagnostic) {
+ llvm::StringSet<> Result;
+ if (!Diagnostic.consume_front("invalid configuration value '") ||
+ !Diagnostic.contains("' for option '")) {
+ Result.insert("");
+ return Result;
+ }
+
+ auto [Value, Rest] = Diagnostic.split("' for option '");
+ auto [Option, _] = Rest.split('\'');
+
+ for (const auto &[Opts, Source] : RawOptions) {
+ for (const auto &[ItOption, ItValue] : Opts.CheckOptions) {
+ if (ItOption == Option && ItValue.Value == Value) {
+ Result.insert(Source);
+ break;
+ }
+ }
+ }
+
+ if (Result.empty())
+ Result.insert("");
+
+ return Result;
+}
+
+static bool verifyDiagnosticErrors(
+ const std::vector<ClangTidyError> &Errors,
+ const std::vector<ClangTidyOptionsProvider::OptionsSource> &RawOptions) {
+ for (const ClangTidyError &Error : Errors) {
+ for (const auto &Source :
+ findSourcesForDiagnostic(RawOptions, Error.Message.Message)) {
+ getDiagnosticForLevel(Error.DiagLevel, Source.getKey())
+ << Error.Message.Message << " [" << Error.DiagnosticName << "]\n";
+ for (const DiagnosticMessage &Note : Error.Notes)
+ llvm::WithColor::note(llvm::errs(), Source.getKey())
+ << Note.Message << '\n';
+ }
+ }
+
+ return !Errors.empty();
+}
+
int clangTidyMain(int argc, const char **argv) {
llvm::InitLLVM X(argc, argv);
@@ -607,9 +664,10 @@
if (VerifyConfig) {
std::vector<ClangTidyOptionsProvider::OptionsSource> RawOptions =
OptionsProvider->getRawOptions(FileName);
- NamesAndOptions Valid =
- getAllChecksAndOptions(AllowEnablingAnalyzerAlphaCheckers);
+ NamesAndOptions Valid = getAllChecksAndOptions(
+ EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
bool AnyInvalid = false;
+ AnyInvalid |= verifyDiagnosticErrors(Valid.Errors, RawOptions);
for (const auto &[Opts, Source] : RawOptions) {
if (Opts.Checks)
AnyInvalid |= verifyChecks(Valid.Names, *Opts.Checks, Source);
Index: clang-tools-extra/clang-tidy/ClangTidy.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.h
+++ clang-tools-extra/clang-tidy/ClangTidy.h
@@ -61,10 +61,12 @@
struct NamesAndOptions {
llvm::StringSet<> Names;
llvm::StringSet<> Options;
+ std::vector<ClangTidyError> Errors;
};
NamesAndOptions
-getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers = true);
+getAllChecksAndOptions(const ClangTidyOptions &Options,
+ bool AllowEnablingAnalyzerAlphaCheckers = true);
/// Returns the effective check-specific options.
///
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -498,6 +498,10 @@
std::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(),
Options),
AllowEnablingAnalyzerAlphaCheckers);
+ ClangTidyDiagnosticConsumer DiagConsumer(Context, nullptr, false, false);
+ DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions(),
+ &DiagConsumer, /*ShouldOwnClient=*/false);
+ Context.setDiagnosticsEngine(&DE);
ClangTidyASTConsumerFactory Factory(Context);
return Factory.getCheckOptions();
}
@@ -623,13 +627,18 @@
}
NamesAndOptions
-getAllChecksAndOptions(bool AllowEnablingAnalyzerAlphaCheckers) {
+getAllChecksAndOptions(const ClangTidyOptions &Options,
+ bool AllowEnablingAnalyzerAlphaCheckers) {
NamesAndOptions Result;
- ClangTidyOptions Opts;
- Opts.Checks = "*";
+ ClangTidyOptions Opt(Options);
+ Opt.Checks = "*";
clang::tidy::ClangTidyContext Context(
- std::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(), Opts),
+ std::make_unique<DefaultOptionsProvider>(ClangTidyGlobalOptions(), Opt),
AllowEnablingAnalyzerAlphaCheckers);
+ ClangTidyDiagnosticConsumer DiagConsumer(Context, nullptr, false, false);
+ DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions(),
+ &DiagConsumer, /*ShouldOwnClient=*/false);
+ Context.setDiagnosticsEngine(&DE);
ClangTidyCheckFactories Factories;
for (const ClangTidyModuleRegistry::entry &Module :
ClangTidyModuleRegistry::entries()) {
@@ -655,6 +664,7 @@
Factory.getValue()(Factory.getKey(), &Context);
}
+ Result.Errors = DiagConsumer.take();
return Result;
}
} // namespace clang::tidy
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
