https://github.com/galenelias updated
https://github.com/llvm/llvm-project/pull/136167
>From f54d980bafd76863ab63e68f0568dcfc3329fd9f Mon Sep 17 00:00:00 2001
From: Galen Elias <gel...@gmail.com>
Date: Wed, 16 Apr 2025 12:48:18 -0700
Subject: [PATCH 1/6] [clang-tidy] Return error code on config parse error
This addresses:
Issue 77341: clang-tidy fails to parse config files silently, even with
--verify-config.
Currently when clang-tidy attempts to parse a .clang-tidy config file
with invalid syntax, it emits and error and moves on, which can often
result in applying the default set of checks, which is undesirable.
This change bubbles up the error state from parsing the configuration
files up to `clangTidyMain` so we can do an early return with an error
code. It's unfortunately quite a bit of plumbing - but this seemed to be
the idiomatic approach for the clang-tidy codebase.
---
.../ClangTidyDiagnosticConsumer.cpp | 2 +-
.../clang-tidy/ClangTidyOptions.cpp | 66 +++++++++++--------
.../clang-tidy/ClangTidyOptions.h | 23 ++++---
.../clang-tidy/tool/ClangTidyMain.cpp | 47 +++++++------
.../clang-tidy/OptionsProviderTest.cpp | 16 ++---
5 files changed, 91 insertions(+), 63 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 731141a545a48..4691b1606986b 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -266,7 +266,7 @@ ClangTidyOptions
ClangTidyContext::getOptionsForFile(StringRef File) const {
// Merge options on top of getDefaults() as a safeguard against options with
// unset values.
return ClangTidyOptions::getDefaults().merge(
- OptionsProvider->getOptions(File), 0);
+ *OptionsProvider->getOptions(File), 0);
}
void ClangTidyContext::setEnableProfiling(bool P) { Profile = P; }
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
index dd1d86882f5d4..7ae23c137106e 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
@@ -267,16 +267,19 @@ const char
ClangTidyOptionsProvider::OptionsSourceTypeConfigCommandLineOption[] =
"command-line option '-config'";
-ClangTidyOptions
+llvm::ErrorOr<ClangTidyOptions>
ClangTidyOptionsProvider::getOptions(llvm::StringRef FileName) {
ClangTidyOptions Result;
unsigned Priority = 0;
- for (auto &Source : getRawOptions(FileName))
+ llvm::ErrorOr<std::vector<OptionsSource>> Options = getRawOptions(FileName);
+ if (!Options)
+ return Options.getError();
+ for (auto &Source : *Options)
Result.mergeWith(Source.first, ++Priority);
return Result;
}
-std::vector<OptionsSource>
+llvm::ErrorOr<std::vector<OptionsSource>>
DefaultOptionsProvider::getRawOptions(llvm::StringRef FileName) {
std::vector<OptionsSource> Result;
Result.emplace_back(DefaultOptions, OptionsSourceTypeDefaultBinary);
@@ -292,10 +295,12 @@ ConfigOptionsProvider::ConfigOptionsProvider(
std::move(OverrideOptions), std::move(FS)),
ConfigOptions(std::move(ConfigOptions)) {}
-std::vector<OptionsSource>
+llvm::ErrorOr<std::vector<OptionsSource>>
ConfigOptionsProvider::getRawOptions(llvm::StringRef FileName) {
- std::vector<OptionsSource> RawOptions =
+ llvm::ErrorOr<std::vector<OptionsSource>> RawOptions =
DefaultOptionsProvider::getRawOptions(FileName);
+ if (!RawOptions)
+ return RawOptions;
if (ConfigOptions.InheritParentConfig.value_or(false)) {
LLVM_DEBUG(llvm::dbgs()
<< "Getting options for file " << FileName << "...\n");
@@ -303,13 +308,13 @@ ConfigOptionsProvider::getRawOptions(llvm::StringRef
FileName) {
llvm::ErrorOr<llvm::SmallString<128>> AbsoluteFilePath =
getNormalizedAbsolutePath(FileName);
if (AbsoluteFilePath) {
- addRawFileOptions(AbsoluteFilePath->str(), RawOptions);
+ addRawFileOptions(AbsoluteFilePath->str(), *RawOptions);
}
}
- RawOptions.emplace_back(ConfigOptions,
- OptionsSourceTypeConfigCommandLineOption);
- RawOptions.emplace_back(OverrideOptions,
- OptionsSourceTypeCheckCommandLineOption);
+ RawOptions->emplace_back(ConfigOptions,
+ OptionsSourceTypeConfigCommandLineOption);
+ RawOptions->emplace_back(OverrideOptions,
+ OptionsSourceTypeCheckCommandLineOption);
return RawOptions;
}
@@ -345,21 +350,21 @@
FileOptionsBaseProvider::getNormalizedAbsolutePath(llvm::StringRef Path) {
return NormalizedAbsolutePath;
}
-void FileOptionsBaseProvider::addRawFileOptions(
+std::error_code FileOptionsBaseProvider::addRawFileOptions(
llvm::StringRef AbsolutePath, std::vector<OptionsSource> &CurOptions) {
auto CurSize = CurOptions.size();
// Look for a suitable configuration file in all parent directories of the
// file. Start with the immediate parent directory and move up.
StringRef RootPath = llvm::sys::path::parent_path(AbsolutePath);
auto MemorizedConfigFile =
- [this, &RootPath](StringRef CurrentPath) -> std::optional<OptionsSource>
{
+ [this, &RootPath](StringRef CurrentPath) -> llvm::ErrorOr<OptionsSource>
{
const auto Iter = CachedOptions.Memorized.find(CurrentPath);
if (Iter != CachedOptions.Memorized.end())
return CachedOptions.Storage[Iter->second];
- std::optional<OptionsSource> OptionsSource =
tryReadConfigFile(CurrentPath);
+ llvm::ErrorOr<OptionsSource> OptionsSource =
tryReadConfigFile(CurrentPath);
if (OptionsSource) {
const size_t Index = CachedOptions.Storage.size();
- CachedOptions.Storage.emplace_back(OptionsSource.value());
+ CachedOptions.Storage.emplace_back(*OptionsSource);
while (RootPath != CurrentPath) {
LLVM_DEBUG(llvm::dbgs()
<< "Caching configuration for path " << RootPath << ".\n");
@@ -373,16 +378,21 @@ void FileOptionsBaseProvider::addRawFileOptions(
};
for (StringRef CurrentPath = RootPath; !CurrentPath.empty();
CurrentPath = llvm::sys::path::parent_path(CurrentPath)) {
- if (std::optional<OptionsSource> Result =
- MemorizedConfigFile(CurrentPath)) {
- CurOptions.emplace_back(Result.value());
+ llvm::ErrorOr<OptionsSource> Result = MemorizedConfigFile(CurrentPath);
+
+ if (Result) {
+ CurOptions.emplace_back(*Result);
if (!Result->first.InheritParentConfig.value_or(false))
break;
+ } else if (Result.getError() != llvm::errc::no_such_file_or_directory) {
+ return Result.getError();
}
}
// Reverse order of file configs because closer configs should have higher
// priority.
std::reverse(CurOptions.begin() + CurSize, CurOptions.end());
+
+ return {};
}
FileOptionsProvider::FileOptionsProvider(
@@ -404,7 +414,7 @@ FileOptionsProvider::FileOptionsProvider(
// FIXME: This method has some common logic with clang::format::getStyle().
// Consider pulling out common bits to a findParentFileWithName function or
// similar.
-std::vector<OptionsSource>
+llvm::ErrorOr<std::vector<OptionsSource>>
FileOptionsProvider::getRawOptions(StringRef FileName) {
LLVM_DEBUG(llvm::dbgs() << "Getting options for file " << FileName
<< "...\n");
@@ -412,19 +422,23 @@ FileOptionsProvider::getRawOptions(StringRef FileName) {
llvm::ErrorOr<llvm::SmallString<128>> AbsoluteFilePath =
getNormalizedAbsolutePath(FileName);
if (!AbsoluteFilePath)
- return {};
+ return std::vector<OptionsSource>{};
- std::vector<OptionsSource> RawOptions =
+ llvm::ErrorOr<std::vector<OptionsSource>> RawOptions =
DefaultOptionsProvider::getRawOptions(AbsoluteFilePath->str());
- addRawFileOptions(AbsoluteFilePath->str(), RawOptions);
+ std::error_code Err = addRawFileOptions(AbsoluteFilePath->str(),
*RawOptions);
+
+ if (Err)
+ return Err;
+
OptionsSource CommandLineOptions(OverrideOptions,
OptionsSourceTypeCheckCommandLineOption);
- RawOptions.push_back(CommandLineOptions);
+ RawOptions->push_back(CommandLineOptions);
return RawOptions;
}
-std::optional<OptionsSource>
+llvm::ErrorOr<OptionsSource>
FileOptionsBaseProvider::tryReadConfigFile(StringRef Directory) {
assert(!Directory.empty());
@@ -433,7 +447,7 @@ FileOptionsBaseProvider::tryReadConfigFile(StringRef
Directory) {
if (!DirectoryStatus || !DirectoryStatus->isDirectory()) {
llvm::errs() << "Error reading configuration from " << Directory
<< ": directory doesn't exist.\n";
- return std::nullopt;
+ return make_error_code(llvm::errc::not_a_directory);
}
for (const ConfigFileHandler &ConfigHandler : ConfigHandlers) {
@@ -464,11 +478,11 @@ FileOptionsBaseProvider::tryReadConfigFile(StringRef
Directory) {
if (ParsedOptions.getError())
llvm::errs() << "Error parsing " << ConfigFile << ": "
<< ParsedOptions.getError().message() << "\n";
- continue;
+ return ParsedOptions.getError();
}
return OptionsSource(*ParsedOptions, std::string(ConfigFile));
}
- return std::nullopt;
+ return make_error_code(llvm::errc::no_such_file_or_directory);
}
/// Parses -line-filter option and stores it to the \c Options.
diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h
b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
index dd78c570d25d9..83162e7c78d5a 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -174,12 +174,12 @@ class ClangTidyOptionsProvider {
/// Returns an ordered vector of OptionsSources, in order of increasing
/// priority.
- virtual std::vector<OptionsSource>
+ virtual llvm::ErrorOr<std::vector<OptionsSource>>
getRawOptions(llvm::StringRef FileName) = 0;
/// Returns options applying to a specific translation unit with the
/// specified \p FileName.
- ClangTidyOptions getOptions(llvm::StringRef FileName);
+ llvm::ErrorOr<ClangTidyOptions> getOptions(llvm::StringRef FileName);
};
/// Implementation of the \c ClangTidyOptionsProvider interface, which
@@ -193,7 +193,8 @@ class DefaultOptionsProvider : public
ClangTidyOptionsProvider {
const ClangTidyGlobalOptions &getGlobalOptions() override {
return GlobalOptions;
}
- std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
+ llvm::ErrorOr<std::vector<OptionsSource>>
+ getRawOptions(llvm::StringRef FileName) override;
private:
ClangTidyGlobalOptions GlobalOptions;
@@ -204,7 +205,9 @@ class FileOptionsBaseProvider : public
DefaultOptionsProvider {
protected:
// A pair of configuration file base name and a function parsing
// configuration from text in the corresponding format.
- using ConfigFileHandler = std::pair<std::string,
std::function<llvm::ErrorOr<ClangTidyOptions> (llvm::MemoryBufferRef)>>;
+ using ConfigFileHandler =
+ std::pair<std::string, std::function<llvm::ErrorOr<ClangTidyOptions>(
+ llvm::MemoryBufferRef)>>;
/// Configuration file handlers listed in the order of priority.
///
@@ -235,15 +238,15 @@ class FileOptionsBaseProvider : public
DefaultOptionsProvider {
ClangTidyOptions OverrideOptions,
ConfigFileHandlers ConfigHandlers);
- void addRawFileOptions(llvm::StringRef AbsolutePath,
- std::vector<OptionsSource> &CurOptions);
+ std::error_code addRawFileOptions(llvm::StringRef AbsolutePath,
+ std::vector<OptionsSource> &CurOptions);
llvm::ErrorOr<llvm::SmallString<128>>
getNormalizedAbsolutePath(llvm::StringRef AbsolutePath);
/// Try to read configuration files from \p Directory using registered
/// \c ConfigHandlers.
- std::optional<OptionsSource> tryReadConfigFile(llvm::StringRef Directory);
+ llvm::ErrorOr<OptionsSource> tryReadConfigFile(llvm::StringRef Directory);
struct OptionsCache {
llvm::StringMap<size_t> Memorized;
@@ -262,7 +265,8 @@ class ConfigOptionsProvider : public
FileOptionsBaseProvider {
ClangTidyGlobalOptions GlobalOptions, ClangTidyOptions DefaultOptions,
ClangTidyOptions ConfigOptions, ClangTidyOptions OverrideOptions,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS = nullptr);
- std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
+ llvm::ErrorOr<std::vector<OptionsSource>>
+ getRawOptions(llvm::StringRef FileName) override;
private:
ClangTidyOptions ConfigOptions;
@@ -315,7 +319,8 @@ class FileOptionsProvider : public FileOptionsBaseProvider {
ClangTidyOptions OverrideOptions,
ConfigFileHandlers ConfigHandlers);
- std::vector<OptionsSource> getRawOptions(llvm::StringRef FileName) override;
+ llvm::ErrorOr<std::vector<OptionsSource>>
+ getRawOptions(llvm::StringRef FileName) override;
};
/// Parses LineFilter from JSON and stores it to the \p Options.
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index fa8887e4639b4..4d7b511b0d1ac 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -337,8 +337,7 @@ Allow empty enabled checks. This suppresses
the "no checks enabled" error when disabling
all of the checks.
)"),
- cl::init(false),
- cl::cat(ClangTidyCategory));
+ cl::init(false),
cl::cat(ClangTidyCategory));
namespace clang::tidy {
@@ -370,8 +369,8 @@ static void printStats(const ClangTidyStats &Stats) {
}
}
-static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
- llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) {
+static std::unique_ptr<ClangTidyOptionsProvider>
+createOptionsProvider(llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) {
ClangTidyGlobalOptions GlobalOptions;
if (std::error_code Err = parseLineFilter(LineFilter, GlobalOptions)) {
llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n";
@@ -624,21 +623,29 @@ int clangTidyMain(int argc, const char **argv) {
}
SmallString<256> FilePath = makeAbsolute(FileName);
- ClangTidyOptions EffectiveOptions = OptionsProvider->getOptions(FilePath);
+ llvm::ErrorOr<ClangTidyOptions> EffectiveOptions =
+ OptionsProvider->getOptions(FilePath);
+
+ if (!EffectiveOptions)
+ return 1;
std::vector<std::string> EnabledChecks =
- getCheckNames(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
+ getCheckNames(*EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
if (ExplainConfig) {
// FIXME: Show other ClangTidyOptions' fields, like ExtraArg.
- std::vector<clang::tidy::ClangTidyOptionsProvider::OptionsSource>
+ llvm::ErrorOr<
+ std::vector<clang::tidy::ClangTidyOptionsProvider::OptionsSource>>
RawOptions = OptionsProvider->getRawOptions(FilePath);
- for (const std::string &Check : EnabledChecks) {
- for (const auto &[Opts, Source] : llvm::reverse(RawOptions)) {
- if (Opts.Checks && GlobList(*Opts.Checks).contains(Check)) {
- llvm::outs() << "'" << Check << "' is enabled in the " << Source
- << ".\n";
- break;
+
+ if (RawOptions) {
+ for (const std::string &Check : EnabledChecks) {
+ for (const auto &[Opts, Source] : llvm::reverse(*RawOptions)) {
+ if (Opts.Checks && GlobList(*Opts.Checks).contains(Check)) {
+ llvm::outs() << "'" << Check << "' is enabled in the " << Source
+ << ".\n";
+ break;
+ }
}
}
}
@@ -658,21 +665,23 @@ int clangTidyMain(int argc, const char **argv) {
}
if (DumpConfig) {
- EffectiveOptions.CheckOptions =
- getCheckOptions(EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
+ EffectiveOptions->CheckOptions =
+ getCheckOptions(*EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
llvm::outs() << configurationAsText(ClangTidyOptions::getDefaults().merge(
- EffectiveOptions, 0))
+ *EffectiveOptions, 0))
<< "\n";
return 0;
}
if (VerifyConfig) {
- std::vector<ClangTidyOptionsProvider::OptionsSource> RawOptions =
- OptionsProvider->getRawOptions(FileName);
+ llvm::ErrorOr<std::vector<ClangTidyOptionsProvider::OptionsSource>>
+ RawOptions = OptionsProvider->getRawOptions(FileName);
+ if (!RawOptions)
+ return 1;
ChecksAndOptions Valid =
getAllChecksAndOptions(AllowEnablingAnalyzerAlphaCheckers);
bool AnyInvalid = false;
- for (const auto &[Opts, Source] : RawOptions) {
+ for (const auto &[Opts, Source] : *RawOptions) {
if (Opts.Checks)
AnyInvalid |= verifyChecks(Valid.Checks, *Opts.Checks, Source);
if (Opts.HeaderFileExtensions && Opts.ImplementationFileExtensions)
diff --git a/clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp
b/clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp
index 5aa3730ac5ccf..427d4e42ff567 100644
--- a/clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp
@@ -46,20 +46,20 @@ TEST(ClangTidyOptionsProvider, InMemoryFileSystems) {
FileOptionsProvider FileOpt({}, {}, {}, FileSystem);
- ClangTidyOptions File1Options =
+ llvm::ErrorOr<ClangTidyOptions> File1Options =
FileOpt.getOptions("ProjectRoot/SubDir1/File.cpp");
- ClangTidyOptions File2Options =
+ llvm::ErrorOr<ClangTidyOptions> File2Options =
FileOpt.getOptions("ProjectRoot/SubDir1/SubDir2/File.cpp");
- ClangTidyOptions File3Options =
+ llvm::ErrorOr<ClangTidyOptions> File3Options =
FileOpt.getOptions("ProjectRoot/SubDir1/SubDir2/SubDir3/File.cpp");
- ASSERT_TRUE(File1Options.Checks.has_value());
- EXPECT_EQ(*File1Options.Checks, "-*,clang-diagnostic-*,readability-*");
- ASSERT_TRUE(File2Options.Checks.has_value());
- EXPECT_EQ(*File2Options.Checks, "bugprone-*,misc-*,clang-diagnostic-*");
+ ASSERT_TRUE(File1Options->Checks.has_value());
+ EXPECT_EQ(*File1Options->Checks, "-*,clang-diagnostic-*,readability-*");
+ ASSERT_TRUE(File2Options->Checks.has_value());
+ EXPECT_EQ(*File2Options->Checks, "bugprone-*,misc-*,clang-diagnostic-*");
// 2 and 3 should use the same config so these should also be the same.
- EXPECT_EQ(File2Options.Checks, File3Options.Checks);
+ EXPECT_EQ(File2Options->Checks, File3Options->Checks);
}
} // namespace test
>From c1f80b8f9124d2ad0fd7564d246f746c88442069 Mon Sep 17 00:00:00 2001
From: Galen Elias <gel...@gmail.com>
Date: Thu, 1 May 2025 10:11:47 -0700
Subject: [PATCH 2/6] Add Release Notes
---
clang-tools-extra/docs/ReleaseNotes.rst | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 761c1d3a80359..a4207bcf220ce 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -103,6 +103,9 @@ Improvements to clang-tidy
- Fixed bug in :program:`clang-tidy` by which `HeaderFilterRegex` did not take
effect when passed via the `.clang-tidy` file.
+- Changed :program:`clang-tidy` to exit with an error code if it encounters an
+ invalid configuration file.
+
New checks
^^^^^^^^^^
>From 8d4a9fa195eb867b306e9c3c3084029143447f63 Mon Sep 17 00:00:00 2001
From: Galen Elias <gel...@gmail.com>
Date: Thu, 1 May 2025 10:12:36 -0700
Subject: [PATCH 3/6] Add some Unit Test coverage
---
.../clang-tidy/OptionsProviderTest.cpp | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp
b/clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp
index 427d4e42ff567..63c03489b0f41 100644
--- a/clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/OptionsProviderTest.cpp
@@ -62,6 +62,51 @@ TEST(ClangTidyOptionsProvider, InMemoryFileSystems) {
EXPECT_EQ(File2Options->Checks, File3Options->Checks);
}
+TEST(ClangTidyOptionsProvider, InvalidConfigurationFiles) {
+ llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> FileSystem(
+ new llvm::vfs::InMemoryFileSystem);
+
+ StringRef BaseClangTidy = R"(
+ Checks: -*,clang-diagnostic-*
+ Unexpected
+ )";
+ StringRef Sub1ClangTidy = R"(
+ Checks: readability-*
+ InheritParentConfig: true
+ )";
+ StringRef Sub2ClangTidy = R"(
+ Checks: bugprone-*,misc-*,clang-diagnostic-*
+ InheritParentConfig: false
+ )";
+ FileSystem->addFile("ProjectRoot/.clang-tidy", 0,
+ llvm::MemoryBuffer::getMemBuffer(BaseClangTidy));
+ FileSystem->addFile("ProjectRoot/File.cpp", 0,
+ llvm::MemoryBuffer::getMemBuffer(""));
+ FileSystem->addFile("ProjectRoot/SubDir1/.clang-tidy", 0,
+ llvm::MemoryBuffer::getMemBuffer(Sub1ClangTidy));
+ FileSystem->addFile("ProjectRoot/SubDir1/File.cpp", 0,
+ llvm::MemoryBuffer::getMemBuffer(""));
+ FileSystem->addFile("ProjectRoot/SubDir1/SubDir2/.clang-tidy", 0,
+ llvm::MemoryBuffer::getMemBuffer(Sub2ClangTidy));
+ FileSystem->addFile("ProjectRoot/SubDir1/SubDir2/File.cpp", 0,
+ llvm::MemoryBuffer::getMemBuffer(""));
+
+ FileOptionsProvider FileOpt({}, {}, {}, FileSystem);
+
+ llvm::ErrorOr<ClangTidyOptions> File1Options =
+ FileOpt.getOptions("ProjectRoot/File.cpp");
+ llvm::ErrorOr<ClangTidyOptions> File2Options =
+ FileOpt.getOptions("ProjectRoot/SubDir1/File.cpp");
+ llvm::ErrorOr<ClangTidyOptions> File3Options =
+ FileOpt.getOptions("ProjectRoot/SubDir1/SubDir2/File.cpp");
+
+ ASSERT_TRUE(!File1Options);
+ ASSERT_TRUE(!File2Options);
+ ASSERT_TRUE(!!File3Options);
+ ASSERT_TRUE(File3Options->Checks.has_value());
+ EXPECT_EQ(*File3Options->Checks, "bugprone-*,misc-*,clang-diagnostic-*");
+}
+
} // namespace test
} // namespace tidy
} // namespace clang
>From 550e63b7999432a99f92e8fd15334ceb95dce4dc Mon Sep 17 00:00:00 2001
From: Galen Elias <gel...@gmail.com>
Date: Thu, 1 May 2025 10:13:05 -0700
Subject: [PATCH 4/6] Revert unrelated clang-format fixes
---
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 4d7b511b0d1ac..1214bb0487b5d 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -337,7 +337,8 @@ Allow empty enabled checks. This suppresses
the "no checks enabled" error when disabling
all of the checks.
)"),
- cl::init(false),
cl::cat(ClangTidyCategory));
+ cl::init(false),
+ cl::cat(ClangTidyCategory));
namespace clang::tidy {
@@ -369,8 +370,8 @@ static void printStats(const ClangTidyStats &Stats) {
}
}
-static std::unique_ptr<ClangTidyOptionsProvider>
-createOptionsProvider(llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) {
+static std::unique_ptr<ClangTidyOptionsProvider> createOptionsProvider(
+ llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) {
ClangTidyGlobalOptions GlobalOptions;
if (std::error_code Err = parseLineFilter(LineFilter, GlobalOptions)) {
llvm::errs() << "Invalid LineFilter: " << Err.message() << "\n\nUsage:\n";
>From 64b4b67efcd7d3380e08c134e1f8c4d44dda7c8e Mon Sep 17 00:00:00 2001
From: Galen Elias <gel...@gmail.com>
Date: Thu, 1 May 2025 10:14:08 -0700
Subject: [PATCH 5/6] Return error code in ExplainConfig path
---
.../clang-tidy/tool/ClangTidyMain.cpp | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 1214bb0487b5d..6536ad76846c8 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -639,14 +639,15 @@ int clangTidyMain(int argc, const char **argv) {
std::vector<clang::tidy::ClangTidyOptionsProvider::OptionsSource>>
RawOptions = OptionsProvider->getRawOptions(FilePath);
- if (RawOptions) {
- for (const std::string &Check : EnabledChecks) {
- for (const auto &[Opts, Source] : llvm::reverse(*RawOptions)) {
- if (Opts.Checks && GlobList(*Opts.Checks).contains(Check)) {
- llvm::outs() << "'" << Check << "' is enabled in the " << Source
- << ".\n";
- break;
- }
+ if (!RawOptions)
+ return 1;
+
+ for (const std::string &Check : EnabledChecks) {
+ for (const auto &[Opts, Source] : llvm::reverse(*RawOptions)) {
+ if (Opts.Checks && GlobList(*Opts.Checks).contains(Check)) {
+ llvm::outs() << "'" << Check << "' is enabled in the " << Source
+ << ".\n";
+ break;
}
}
}
>From 2e09fe1611edcfcd8a2d6f370d1e4d991bbe7808 Mon Sep 17 00:00:00 2001
From: Galen Elias <gel...@gmail.com>
Date: Thu, 1 May 2025 13:36:50 -0700
Subject: [PATCH 6/6] Validate configs for all inputs files up front
And make ClangTidyContext::getOptionsForFile resilient to errors at that
point.
---
.../clang-tidy/ClangTidyDiagnosticConsumer.cpp | 12 ++++++++++--
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp | 11 +++++++++++
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 4691b1606986b..befd8e1b36637 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -265,8 +265,16 @@ const ClangTidyOptions &ClangTidyContext::getOptions()
const {
ClangTidyOptions ClangTidyContext::getOptionsForFile(StringRef File) const {
// Merge options on top of getDefaults() as a safeguard against options with
// unset values.
- return ClangTidyOptions::getDefaults().merge(
- *OptionsProvider->getOptions(File), 0);
+ ClangTidyOptions defaultOptions = ClangTidyOptions::getDefaults();
+ llvm::ErrorOr<ClangTidyOptions> fileOptions =
+ OptionsProvider->getOptions(File);
+
+ // If there was an error parsing the options, just use the default options.
+ // Ideally, the options for each file should be validated before this point.
+ if (!fileOptions)
+ return defaultOptions;
+
+ return defaultOptions.merge(*fileOptions, 0);
}
void ClangTidyContext::setEnableProfiling(bool P) { Profile = P; }
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 6536ad76846c8..021e6c7f21f37 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -630,6 +630,17 @@ int clangTidyMain(int argc, const char **argv) {
if (!EffectiveOptions)
return 1;
+ // Validate the configuration files associated with all input files so we can
+ // return an error up front.
+ if (PathList.size() > 1) {
+ for (auto iter = PathList.begin() + 1; iter != PathList.end(); ++iter) {
+ llvm::ErrorOr<ClangTidyOptions> Options =
+ OptionsProvider->getOptions(*iter);
+ if (!Options)
+ return 1;
+ }
+ }
+
std::vector<std::string> EnabledChecks =
getCheckNames(*EffectiveOptions, AllowEnablingAnalyzerAlphaCheckers);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
