njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, gribozavr2.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, xazax.hun.
Herald added a project: clang.
njames93 requested review of this revision.
There was already a basic version of this in IdentifierNamingCheck, controlled
by an option GetConfigPerFile. This has now been changed to a global option
UsePerFileConfig. As there hasn't been a release with GetConfigPerFile, This
change shouldn't break anyones config.
Now there is support for this built into the base class directly, along with
some basic caching.
As the clang-tidy providers used only have at best directory resolution, this
caches based on directory rather than filename. This can potentially save alot
of redundant config lookups.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D92175
Files:
clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
clang-tools-extra/clang-tidy/ClangTidyCheck.h
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
clang-tools-extra/clangd/TidyProvider.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-multiple-styles.cpp
@@ -14,7 +14,7 @@
// RUN: -config='{ InheritParentConfig: true, CheckOptions: [ \
// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
-// RUN: {key: readability-identifier-naming.GetConfigPerFile, value: true} \
+// RUN: {key: readability-identifier-naming.UsePerFileConfig, value: true} \
// RUN: ]}' -header-filter='.*' -- -I%theaders
// On DISABLED run, everything should be made 'camelBack'.
@@ -25,7 +25,7 @@
// RUN: -config='{ InheritParentConfig: false, CheckOptions: [ \
// RUN: {key: readability-identifier-naming.FunctionCase, value: camelBack}, \
// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
-// RUN: {key: readability-identifier-naming.GetConfigPerFile, value: false} \
+// RUN: {key: readability-identifier-naming.UsePerFileConfig, value: false} \
// RUN: ]}' -header-filter='.*' -- -I%theaders
#include "global-style1/header.h"
Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -51,7 +51,6 @@
- :option:`EnumCase`, :option:`EnumPrefix`, :option:`EnumSuffix`, :option:`EnumIgnoredRegexp`
- :option:`EnumConstantCase`, :option:`EnumConstantPrefix`, :option:`EnumConstantSuffix`, :option:`EnumConstantIgnoredRegexp`
- :option:`FunctionCase`, :option:`FunctionPrefix`, :option:`FunctionSuffix`, :option:`FunctionIgnoredRegexp`
- - :option:`GetConfigPerFile`
- :option:`GlobalConstantCase`, :option:`GlobalConstantPrefix`, :option:`GlobalConstantSuffix`, :option:`GlobalConstantIgnoredRegexp`
- :option:`GlobalConstantPointerCase`, :option:`GlobalConstantPointerPrefix`, :option:`GlobalConstantPointerSuffix`, :option:`GlobalConstantPointerIgnoredRegexp`
- :option:`GlobalFunctionCase`, :option:`GlobalFunctionPrefix`, :option:`GlobalFunctionSuffix`, :option:`GlobalFunctionIgnoredRegexp`
@@ -86,6 +85,7 @@
- :option:`TypedefCase`, :option:`TypedefPrefix`, :option:`TypedefSuffix`, :option:`TypedefIgnoredRegexp`
- :option:`TypeTemplateParameterCase`, :option:`TypeTemplateParameterPrefix`, :option:`TypeTemplateParameterSuffix`, :option:`TypeTemplateParameterIgnoredRegexp`
- :option:`UnionCase`, :option:`UnionPrefix`, :option:`UnionSuffix`, :option:`UnionIgnoredRegexp`
+ - :option:`UsePerFileConfig`
- :option:`ValueTemplateParameterCase`, :option:`ValueTemplateParameterPrefix`, :option:`ValueTemplateParameterSuffix`, :option:`ValueTemplateParameterIgnoredRegexp`
- :option:`VariableCase`, :option:`VariablePrefix`, :option:`VariableSuffix`, :option:`VariableIgnoredRegexp`
- :option:`VirtualMethodCase`, :option:`VirtualMethodPrefix`, :option:`VirtualMethodSuffix`, :option:`VirtualMethodIgnoredRegexp`
@@ -790,13 +790,6 @@
char pre_my_function_string_post();
-.. option:: GetConfigPerFile
-
- When `true` the check will look for the configuration for where an
- identifier is declared. Useful for when included header files use a
- different style.
- Default value is `true`.
-
.. option:: GlobalConstantCase
When defined, the check will ensure global constant names conform to the
@@ -2210,6 +2203,13 @@
char b;
};
+.. option:: UsePerFileConfig
+
+ When `true` the check will look for the configuration for where an
+ identifier is declared. Useful for when included header files use a
+ different style.
+ Default value is `true`.
+
.. option:: ValueTemplateParameterCase
When defined, the check will ensure value template parameter names conform to the
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -140,7 +140,7 @@
- Improved :doc:`readability-identifier-naming
<clang-tidy/checks/readability-identifier-naming>` check.
- Added an option `GetConfigPerFile` to support including files which use
+ Added an option `UsePerFileConfig` to support including files which use
different naming styles.
Now renames overridden virtual methods if the method they override has a
Index: clang-tools-extra/clangd/TidyProvider.cpp
===================================================================
--- clang-tools-extra/clangd/TidyProvider.cpp
+++ clang-tools-extra/clangd/TidyProvider.cpp
@@ -15,6 +15,7 @@
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/Process.h"
#include "llvm/Support/VirtualFileSystem.h"
+#include <limits>
#include <memory>
namespace clang {
@@ -159,6 +160,13 @@
llvm::StringRef) {
if (Opts.Checks && !Opts.Checks->empty())
Opts.Checks->append(DisableList);
+
+ // Force any tidy check that uses UsePreFileConfig to disable it. We only
+ // care about diagnostics in the main file so looking for configs in other
+ // files is redundant.
+ Opts.CheckOptions.insert_or_assign(
+ "UsePerFileConfig", tidy::ClangTidyOptions::ClangTidyValue(
+ "false", std::numeric_limits<unsigned>::max()));
};
}
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -102,9 +102,7 @@
/// StyleKind, for a given directory.
mutable llvm::StringMap<FileStyle> NamingStylesCache;
FileStyle *MainFileStyle;
- ClangTidyContext *const Context;
- const std::string CheckName;
- const bool GetConfigPerFile;
+ const bool UsePerFileConfig;
const bool IgnoreFailedSplit;
};
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -170,8 +170,8 @@
IdentifierNamingCheck::IdentifierNamingCheck(StringRef Name,
ClangTidyContext *Context)
- : RenamerClangTidyCheck(Name, Context), Context(Context), CheckName(Name),
- GetConfigPerFile(Options.get("GetConfigPerFile", true)),
+ : RenamerClangTidyCheck(Name, Context),
+ UsePerFileConfig(Options.getLocalOrGlobal("UsePerFileConfig", true)),
IgnoreFailedSplit(Options.get("IgnoreFailedSplit", false)) {
auto IterAndInserted = NamingStylesCache.try_emplace(
@@ -209,7 +209,7 @@
Options.store(Opts, StyleString, *Styles[I]->Case);
}
}
- Options.store(Opts, "GetConfigPerFile", GetConfigPerFile);
+ Options.store(Opts, "UsePerFileConfig", UsePerFileConfig);
Options.store(Opts, "IgnoreFailedSplit", IgnoreFailedSplit);
Options.store(Opts, "IgnoreMainLikeFunctions",
MainFileStyle->isIgnoringMainLikeFunction());
@@ -763,17 +763,16 @@
const IdentifierNamingCheck::FileStyle &
IdentifierNamingCheck::getStyleForFile(StringRef FileName) const {
- if (!GetConfigPerFile)
+ if (!UsePerFileConfig)
return *MainFileStyle;
StringRef Parent = llvm::sys::path::parent_path(FileName);
auto Iter = NamingStylesCache.find(Parent);
if (Iter != NamingStylesCache.end())
return Iter->getValue();
- ClangTidyOptions Options = Context->getOptionsForFile(FileName);
- if (Options.Checks && GlobList(*Options.Checks).contains(CheckName)) {
+ if (isCheckEnabledForFile(FileName)) {
auto It = NamingStylesCache.try_emplace(
- Parent, getFileStyleFromOptions({CheckName, Options.CheckOptions}));
+ Parent, getFileStyleFromOptions(getOptionsForFile(FileName)));
assert(It.second);
return It.first->getValue();
}
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -136,7 +136,7 @@
/// Returns options for \c File. Does not change or depend on
/// \c CurrentFile.
- ClangTidyOptions getOptionsForFile(StringRef File) const;
+ const ClangTidyOptions &getOptionsForFile(StringRef File) const;
/// Returns \c ClangTidyStats containing issued and ignored diagnostic
/// counters.
@@ -190,6 +190,8 @@
std::unique_ptr<CachedGlobList> CheckFilter;
std::unique_ptr<CachedGlobList> WarningAsErrorFilter;
+ mutable llvm::StringMap<ClangTidyOptions> CachedDirectoryOptions;
+
LangOptions LangOpts;
ClangTidyStats Stats;
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -202,11 +202,20 @@
return CurrentOptions;
}
-ClangTidyOptions ClangTidyContext::getOptionsForFile(StringRef File) const {
+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);
+ StringRef Directory = llvm::sys::path::parent_path(File);
+ auto Iter = CachedDirectoryOptions.find(Directory);
+ bool Inserted;
+ if (Iter == CachedDirectoryOptions.end()) {
+ std::tie(Iter, Inserted) = CachedDirectoryOptions.insert_or_assign(
+ Directory, ClangTidyOptions::getDefaults().merge(
+ OptionsProvider->getOptions(File), 0));
+ assert(Inserted);
+ }
+ return Iter->getValue();
}
void ClangTidyContext::setEnableProfiling(bool P) { Profile = P; }
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -500,6 +500,11 @@
StringRef getCurrentMainFile() const { return Context->getCurrentFile(); }
/// Returns the language options from the context.
const LangOptions &getLangOpts() const { return Context->getLangOpts(); }
+ /// Returns the options if clang-tidy had been ran on \p Filename.
+ OptionsView getOptionsForFile(StringRef Filename) const;
+ /// Returns true if this check would be enabled if clang-tidy had been ran on
+ /// \p Filename.
+ bool isCheckEnabledForFile(StringRef Filename) const;
};
/// Read a named option from the ``Context`` and parse it as a bool.
Index: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "ClangTidyCheck.h"
+#include "GlobList.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
@@ -233,5 +234,17 @@
return llvm::None;
}
+ClangTidyCheck::OptionsView
+ClangTidyCheck::getOptionsForFile(StringRef Filename) const {
+ return OptionsView(CheckName,
+ Context->getOptionsForFile(Filename).CheckOptions);
+}
+
+bool ClangTidyCheck::isCheckEnabledForFile(StringRef Filename) const {
+ auto Checks = Context->getOptionsForFile(Filename).Checks;
+ if (!Checks)
+ return false;
+ return GlobList(*Checks).contains(CheckName);
+}
} // namespace tidy
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
