https://github.com/galenelias updated
https://github.com/llvm/llvm-project/pull/136167
>From b8faa7bb02fb4aa0504978dad1a28493fc745786 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 b216970bfbd8c..0413c39bc58eb 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 7c3aee29c146d..c690ea0ca7198 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 6263a3b443ec6714b48a4cd261c7827f2d254e31 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 579fca54924d5..d91cf61ddc93d 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -114,6 +114,9 @@ Improvements to clang-tidy
- Fixed bug in :program:`run_clang_tidy.py` where the program would not
correctly display the checks enabled by the top-level `.clang-tidy` file.
+- Changed :program:`clang-tidy` to exit with an error code if it encounters an
+ invalid configuration file.
+
New checks
^^^^^^^^^^
>From db40a30fe3ed64c0a8890321d2a339c55dd13f14 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 e8d6c28b3f4c2eec86514bb8792318d50621ba72 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 c690ea0ca7198..278009e378bf2 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 c05ed3db5049eaf6d419ce304e350df21ef57ffe 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 278009e378bf2..6dd97b5c86d3d 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 ef6baf790fd30ad7ab4e2b9448d1d736aceb99f6 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 0413c39bc58eb..7fcd2cdc74ca0 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 6dd97b5c86d3d..570990389dfd1 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
