PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, carlosgalvezp, aaron.ballman.
Herald added subscribers: kbarton, xazax.hun, nemanjai.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
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.
Depends on D156024 <https://reviews.llvm.org/D156024>
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D156161
Files:
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/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
Index: clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp
+++ 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=.* --experimental-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=.* --experimental-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=.* --experimental-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=.* --experimental-enable-module-headers-parsing \
// RUN: -- -I %t/ \
// RUN: -fmodules -fimplicit-modules -fno-implicit-module-maps \
// RUN: -fmodule-map-file=%t/module.modulemap \
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -111,6 +111,20 @@
be promoted to errors. For custom error promotion, use `-Werror=<warning>`
on the compiler command-line, irrespective of `Checks` (`--checks=`) settings.
+- 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 hidden command line option
+ `--experimental-enable-module-headers-parsing`.
+
New checks
^^^^^^^^^^
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
@@ -264,9 +264,9 @@
cl::cat(ClangTidyCategory));
static cl::opt<bool>
- DisableModuleHeadersParsing("experimental-disable-module-headers-parsing",
- cl::init(false), cl::Hidden,
- cl::cat(ClangTidyCategory));
+ EnableModuleHeadersParsing("experimental-enable-module-headers-parsing",
+ cl::init(false), cl::Hidden,
+ cl::cat(ClangTidyCategory));
static cl::opt<std::string> ExportFixes("export-fixes", desc(R"(
YAML file to store suggested fixes in. The
@@ -665,7 +665,7 @@
ClangTidyContext Context(std::move(OwningOptionsProvider),
AllowEnablingAnalyzerAlphaCheckers,
- DisableModuleHeadersParsing);
+ EnableModuleHeadersParsing);
std::vector<ClangTidyError> Errors =
runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
FixNotes, EnableCheckProfile, ProfilePrefix);
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -71,7 +71,7 @@
/// Initializes \c ClangTidyContext instance.
ClangTidyContext(std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
bool AllowEnablingAnalyzerAlphaCheckers = false,
- bool DisableModuleHeadersParsing = 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.
@@ -200,7 +200,7 @@
}
bool canEnableModuleHeadersParsing() const {
- return !DisableModuleHeadersParsing;
+ return EnableModuleHeadersParsing;
}
void setSelfContainedDiags(bool Value) { SelfContainedDiags = Value; }
@@ -250,7 +250,7 @@
std::string ProfilePrefix;
bool AllowEnablingAnalyzerAlphaCheckers;
- bool DisableModuleHeadersParsing;
+ bool EnableModuleHeadersParsing;
bool SelfContainedDiags;
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -161,11 +161,11 @@
ClangTidyContext::ClangTidyContext(
std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
- bool AllowEnablingAnalyzerAlphaCheckers, bool DisableModuleHeadersParsing)
+ bool AllowEnablingAnalyzerAlphaCheckers, bool EnableModuleHeadersParsing)
: DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
Profile(false),
AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers),
- DisableModuleHeadersParsing(DisableModuleHeadersParsing),
+ 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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
