[clang-tools-extra] b530eee - [clang-tidy] Add --enable-module-headers-parsing option
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 25 11:31:31 PDT 2023
Author: Piotr Zegar
Date: 2023-07-25T18:31:17Z
New Revision: b530eeea5e7697d58af2adac66c8b86c79e649bb
URL: https://github.com/llvm/llvm-project/commit/b530eeea5e7697d58af2adac66c8b86c79e649bb
DIFF: https://github.com/llvm/llvm-project/commit/b530eeea5e7697d58af2adac66c8b86c79e649bb.diff
LOG: [clang-tidy] Add --enable-module-headers-parsing option
Change default behavior in Clang-tidy and skip by default
module headers parsing. That functionality is buggy and
slow in C++20, and provide tiny benefit.
Reviewed By: carlosgalvezp
Differential Revision: https://reviews.llvm.org/D156161
Added:
Modified:
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/index.rst
clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
index bdead368195e33..3fd1153eb721a9 100644
--- a/clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -418,7 +418,8 @@ ClangTidyASTConsumerFactory::createASTConsumer(
Preprocessor *PP = &Compiler.getPreprocessor();
Preprocessor *ModuleExpanderPP = PP;
- if (Context.getLangOpts().Modules && OverlayFS != nullptr) {
+ if (Context.canEnableModuleHeadersParsing() &&
+ Context.getLangOpts().Modules && OverlayFS != nullptr) {
auto ModuleExpander = std::make_unique<ExpandModularHeadersPPCallbacks>(
&Compiler, OverlayFS);
ModuleExpanderPP = ModuleExpander->getPreprocessor();
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
index 57bd643c00139a..019d9b1f18a5a9 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -161,10 +161,11 @@ ClangTidyError::ClangTidyError(StringRef CheckName,
ClangTidyContext::ClangTidyContext(
std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
- bool AllowEnablingAnalyzerAlphaCheckers)
+ bool AllowEnablingAnalyzerAlphaCheckers, bool EnableModuleHeadersParsing)
: DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
Profile(false),
AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers),
+ EnableModuleHeadersParsing(EnableModuleHeadersParsing),
SelfContainedDiags(false) {
// Before the first translation unit we can get errors related to command-line
// parsing, use empty string for the file name in this case.
diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
index 15f1b672e6d424..d3e15e7b2915ea 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -70,7 +70,8 @@ class ClangTidyContext {
public:
/// Initializes \c ClangTidyContext instance.
ClangTidyContext(std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
- bool AllowEnablingAnalyzerAlphaCheckers = false);
+ bool AllowEnablingAnalyzerAlphaCheckers = false,
+ bool EnableModuleHeadersParsing = false);
/// Sets the DiagnosticsEngine that diag() will emit diagnostics to.
// FIXME: this is required initialization, and should be a constructor param.
// Fix the context -> diag engine -> consumer -> context initialization cycle.
@@ -198,6 +199,12 @@ class ClangTidyContext {
return AllowEnablingAnalyzerAlphaCheckers;
}
+ // This method determines whether preprocessor-level module header parsing is
+ // enabled using the `--experimental-enable-module-headers-parsing` option.
+ bool canEnableModuleHeadersParsing() const {
+ return EnableModuleHeadersParsing;
+ }
+
void setSelfContainedDiags(bool Value) { SelfContainedDiags = Value; }
bool areDiagsSelfContained() const { return SelfContainedDiags; }
@@ -245,6 +252,7 @@ class ClangTidyContext {
std::string ProfilePrefix;
bool AllowEnablingAnalyzerAlphaCheckers;
+ bool EnableModuleHeadersParsing;
bool SelfContainedDiags;
diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
index 74340e1b06cb00..504a174377c69d 100644
--- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -263,6 +263,17 @@ static cl::opt<bool>
cl::init(false), cl::Hidden,
cl::cat(ClangTidyCategory));
+static cl::opt<bool> EnableModuleHeadersParsing("enable-module-headers-parsing",
+ desc(R"(
+Enables preprocessor-level module header parsing
+for C++20 and above, empowering specific checks
+to detect macro definitions within modules. This
+feature may cause performance and parsing issues
+and is therefore considered experimental.
+)"),
+ cl::init(false),
+ cl::cat(ClangTidyCategory));
+
static cl::opt<std::string> ExportFixes("export-fixes", desc(R"(
YAML file to store suggested fixes in. The
stored fixes can be applied to the input source
@@ -659,7 +670,8 @@ int clangTidyMain(int argc, const char **argv) {
llvm::InitializeAllAsmParsers();
ClangTidyContext Context(std::move(OwningOptionsProvider),
- AllowEnablingAnalyzerAlphaCheckers);
+ AllowEnablingAnalyzerAlphaCheckers,
+ EnableModuleHeadersParsing);
std::vector<ClangTidyError> Errors =
runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
FixNotes, EnableCheckProfile, ProfilePrefix);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ef6a8ce2788823..159366222844ec 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -94,6 +94,20 @@ The improvements are...
Improvements to clang-tidy
--------------------------
+- Preprocessor-level module header parsing is now disabled by default due to
+ the problems it caused in C++20 and above, leading to performance and code
+ parsing issues regardless of whether modules were used or not. This change
+ will impact only the following checks:
+ :doc:`modernize-replace-disallow-copy-and-assign-macro
+ <clang-tidy/checks/modernize/replace-disallow-copy-and-assign-macro>`,
+ :doc:`bugprone-reserved-identifier
+ <clang-tidy/checks/bugprone/reserved-identifier>`, and
+ :doc:`readability-identifier-naming
+ <clang-tidy/checks/readability/identifier-naming>`. Those checks will no
+ longer see macros defined in modules. Users can still enable this
+ functionality using the newly added command line option
+ `--enable-module-headers-parsing`.
+
New checks
^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst
index 41fde5064b8eee..852566f2667238 100644
--- a/clang-tools-extra/docs/clang-tidy/index.rst
+++ b/clang-tools-extra/docs/clang-tidy/index.rst
@@ -116,117 +116,122 @@ An overview of all the command-line options:
Generic Options:
- --help - Display available options (--help-hidden for more)
- --help-list - Display list of available options (--help-list-hidden for more)
- --version - Display the version of this program
+ --help - Display available options (--help-hidden for more)
+ --help-list - Display list of available options (--help-list-hidden for more)
+ --version - Display the version of this program
clang-tidy options:
- --checks=<string> - Comma-separated list of globs with optional '-'
- prefix. Globs are processed in order of
- appearance in the list. Globs without '-'
- prefix add checks with matching names to the
- set, globs with the '-' prefix remove checks
- with matching names from the set of enabled
- checks. This option's value is appended to the
- value of the 'Checks' option in .clang-tidy
- file, if any.
- --config=<string> - Specifies a configuration in YAML/JSON format:
- -config="{Checks: '*',
- CheckOptions: {x: y}}"
- When the value is empty, clang-tidy will
- attempt to find a file named .clang-tidy for
- each source file in its parent directories.
- --config-file=<string> - Specify the path of .clang-tidy or custom config file:
- e.g. --config-file=/some/path/myTidyConfigFile
- This option internally works exactly the same way as
- --config option after reading specified config file.
- Use either --config-file or --config, not both.
- --dump-config - Dumps configuration in the YAML format to
- stdout. This option can be used along with a
- file name (and '--' if the file is outside of a
- project with configured compilation database).
- The configuration used for this file will be
- printed.
- Use along with -checks=* to include
- configuration of all checks.
- --enable-check-profile - Enable per-check timing profiles, and print a
- report to stderr.
- --explain-config - For each enabled check explains, where it is
- enabled, i.e. in clang-tidy binary, command
- line or a specific configuration file.
- --export-fixes=<filename> - YAML file to store suggested fixes in. The
- stored fixes can be applied to the input source
- code with clang-apply-replacements.
- --extra-arg=<string> - Additional argument to append to the compiler command line
- --extra-arg-before=<string> - Additional argument to prepend to the compiler command line
- --fix - Apply suggested fixes. Without -fix-errors
- clang-tidy will bail out if any compilation
- errors were found.
- --fix-errors - Apply suggested fixes even if compilation
- errors were found. If compiler errors have
- attached fix-its, clang-tidy will apply them as
- well.
- --fix-notes - If a warning has no fix, but a single fix can
- be found through an associated diagnostic note,
- apply the fix.
- Specifying this flag will implicitly enable the
- '--fix' flag.
- --format-style=<string> - Style for formatting code around applied fixes:
- - 'none' (default) turns off formatting
- - 'file' (literally 'file', not a placeholder)
- uses .clang-format file in the closest parent
- directory
- - '{ <json> }' specifies options inline, e.g.
- -format-style='{BasedOnStyle: llvm, IndentWidth: 8}'
- - 'llvm', 'google', 'webkit', 'mozilla'
- See clang-format documentation for the up-to-date
- information about formatting styles and options.
- This option overrides the 'FormatStyle` option in
- .clang-tidy file, if any.
- --header-filter=<string> - Regular expression matching the names of the
- headers to output diagnostics from. Diagnostics
- from the main file of each translation unit are
- always displayed.
- Can be used together with -line-filter.
- This option overrides the 'HeaderFilterRegex'
- option in .clang-tidy file, if any.
- --line-filter=<string> - List of files with line ranges to filter the
- warnings. Can be used together with
- -header-filter. The format of the list is a
- JSON array of objects:
- [
- {"name":"file1.cpp","lines":[[1,3],[5,7]]},
- {"name":"file2.h"}
- ]
- --list-checks - List all enabled checks and exit. Use with
- -checks=* to list all available checks.
- --load=<pluginfilename> - Load the specified plugin
- -p <string> - Build path
- --quiet - Run clang-tidy in quiet mode. This suppresses
- printing statistics about ignored warnings and
- warnings treated as errors if the respective
- options are specified.
- --store-check-profile=<prefix> - By default reports are printed in tabulated
- format to stderr. When this option is passed,
- these per-TU profiles are instead stored as JSON.
- --system-headers - Display the errors from system headers.
- This option overrides the 'SystemHeaders' option
- in .clang-tidy file, if any.
- --use-color - Use colors in diagnostics. If not set, colors
- will be used if the terminal connected to
- standard output supports colors.
- This option overrides the 'UseColor' option in
- .clang-tidy file, if any.
- --verify-config - Check the config files to ensure each check and
- option is recognized.
- --vfsoverlay=<filename> - Overlay the virtual filesystem described by file
- over the real file system.
- --warnings-as-errors=<string> - Upgrades warnings to errors. Same format as
- '-checks'.
- This option's value is appended to the value of
- the 'WarningsAsErrors' option in .clang-tidy
- file, if any.
+ --checks=<string> - Comma-separated list of globs with optional '-'
+ prefix. Globs are processed in order of
+ appearance in the list. Globs without '-'
+ prefix add checks with matching names to the
+ set, globs with the '-' prefix remove checks
+ with matching names from the set of enabled
+ checks. This option's value is appended to the
+ value of the 'Checks' option in .clang-tidy
+ file, if any.
+ --config=<string> - Specifies a configuration in YAML/JSON format:
+ -config="{Checks: '*',
+ CheckOptions: {x: y}}"
+ When the value is empty, clang-tidy will
+ attempt to find a file named .clang-tidy for
+ each source file in its parent directories.
+ --config-file=<string> - Specify the path of .clang-tidy or custom config file:
+ e.g. --config-file=/some/path/myTidyConfigFile
+ This option internally works exactly the same way as
+ --config option after reading specified config file.
+ Use either --config-file or --config, not both.
+ --dump-config - Dumps configuration in the YAML format to
+ stdout. This option can be used along with a
+ file name (and '--' if the file is outside of a
+ project with configured compilation database).
+ The configuration used for this file will be
+ printed.
+ Use along with -checks=* to include
+ configuration of all checks.
+ --enable-check-profile - Enable per-check timing profiles, and print a
+ report to stderr.
+ --enable-module-headers-parsing - Enables preprocessor-level module header parsing
+ for C++20 and above, empowering specific checks
+ to detect macro definitions within modules. This
+ feature may cause performance and parsing issues
+ and is therefore considered experimental.
+ --explain-config - For each enabled check explains, where it is
+ enabled, i.e. in clang-tidy binary, command
+ line or a specific configuration file.
+ --export-fixes=<filename> - YAML file to store suggested fixes in. The
+ stored fixes can be applied to the input source
+ code with clang-apply-replacements.
+ --extra-arg=<string> - Additional argument to append to the compiler command line
+ --extra-arg-before=<string> - Additional argument to prepend to the compiler command line
+ --fix - Apply suggested fixes. Without -fix-errors
+ clang-tidy will bail out if any compilation
+ errors were found.
+ --fix-errors - Apply suggested fixes even if compilation
+ errors were found. If compiler errors have
+ attached fix-its, clang-tidy will apply them as
+ well.
+ --fix-notes - If a warning has no fix, but a single fix can
+ be found through an associated diagnostic note,
+ apply the fix.
+ Specifying this flag will implicitly enable the
+ '--fix' flag.
+ --format-style=<string> - Style for formatting code around applied fixes:
+ - 'none' (default) turns off formatting
+ - 'file' (literally 'file', not a placeholder)
+ uses .clang-format file in the closest parent
+ directory
+ - '{ <json> }' specifies options inline, e.g.
+ -format-style='{BasedOnStyle: llvm, IndentWidth: 8}'
+ - 'llvm', 'google', 'webkit', 'mozilla'
+ See clang-format documentation for the up-to-date
+ information about formatting styles and options.
+ This option overrides the 'FormatStyle` option in
+ .clang-tidy file, if any.
+ --header-filter=<string> - Regular expression matching the names of the
+ headers to output diagnostics from. Diagnostics
+ from the main file of each translation unit are
+ always displayed.
+ Can be used together with -line-filter.
+ This option overrides the 'HeaderFilterRegex'
+ option in .clang-tidy file, if any.
+ --line-filter=<string> - List of files with line ranges to filter the
+ warnings. Can be used together with
+ -header-filter. The format of the list is a
+ JSON array of objects:
+ [
+ {"name":"file1.cpp","lines":[[1,3],[5,7]]},
+ {"name":"file2.h"}
+ ]
+ --list-checks - List all enabled checks and exit. Use with
+ -checks=* to list all available checks.
+ --load=<pluginfilename> - Load the specified plugin
+ -p <string> - Build path
+ --quiet - Run clang-tidy in quiet mode. This suppresses
+ printing statistics about ignored warnings and
+ warnings treated as errors if the respective
+ options are specified.
+ --store-check-profile=<prefix> - By default reports are printed in tabulated
+ format to stderr. When this option is passed,
+ these per-TU profiles are instead stored as JSON.
+ --system-headers - Display the errors from system headers.
+ This option overrides the 'SystemHeaders' option
+ in .clang-tidy file, if any.
+ --use-color - Use colors in diagnostics. If not set, colors
+ will be used if the terminal connected to
+ standard output supports colors.
+ This option overrides the 'UseColor' option in
+ .clang-tidy file, if any.
+ --verify-config - Check the config files to ensure each check and
+ option is recognized.
+ --vfsoverlay=<filename> - Overlay the virtual filesystem described by file
+ over the real file system.
+ --warnings-as-errors=<string> - Upgrades warnings to errors. Same format as
+ '-checks'.
+ This option's value is appended to the value of
+ the 'WarningsAsErrors' option in .clang-tidy
+ file, if any.
-p <build-path> is used to read a compile command database.
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
index c9b05517d210ce..0422a5cc1b8d75 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
@@ -5,7 +5,7 @@
// RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t/without-modules -- \
// RUN: -config="CheckOptions: [{ \
// RUN: key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN: -header-filter=.* \
+// RUN: -header-filter=.* --enable-module-headers-parsing \
// RUN: -- -I %t/
//
// RUN: rm -rf %t
@@ -14,7 +14,7 @@
// RUN: %check_clang_tidy -std=c++17 %s readability-identifier-naming %t/without-modules -- \
// RUN: -config="CheckOptions: [{ \
// RUN: key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN: -header-filter=.* \
+// RUN: -header-filter=.* --enable-module-headers-parsing \
// RUN: -- -I %t/
//
// Run clang-tidy on a file with modular includes:
@@ -25,7 +25,7 @@
// RUN: %check_clang_tidy -std=c++11 %s readability-identifier-naming %t/with-modules -- \
// RUN: -config="CheckOptions: [{ \
// RUN: key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN: -header-filter=.* \
+// RUN: -header-filter=.* --enable-module-headers-parsing \
// RUN: -- -I %t/ \
// RUN: -fmodules -fimplicit-modules -fno-implicit-module-maps \
// RUN: -fmodule-map-file=%t/module.modulemap \
@@ -37,7 +37,7 @@
// RUN: %check_clang_tidy -std=c++17 %s readability-identifier-naming %t/with-modules -- \
// RUN: -config="CheckOptions: [{ \
// RUN: key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
-// RUN: -header-filter=.* \
+// RUN: -header-filter=.* --enable-module-headers-parsing \
// RUN: -- -I %t/ \
// RUN: -fmodules -fimplicit-modules -fno-implicit-module-maps \
// RUN: -fmodule-map-file=%t/module.modulemap \
More information about the cfe-commits
mailing list