Skip to content

Commit b530eee

Browse files
committed
[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
1 parent 24567dd commit b530eee

File tree

7 files changed

+157
-116
lines changed

7 files changed

+157
-116
lines changed

clang-tools-extra/clang-tidy/ClangTidy.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,8 @@ ClangTidyASTConsumerFactory::createASTConsumer(
418418
Preprocessor *PP = &Compiler.getPreprocessor();
419419
Preprocessor *ModuleExpanderPP = PP;
420420

421-
if (Context.getLangOpts().Modules && OverlayFS != nullptr) {
421+
if (Context.canEnableModuleHeadersParsing() &&
422+
Context.getLangOpts().Modules && OverlayFS != nullptr) {
422423
auto ModuleExpander = std::make_unique<ExpandModularHeadersPPCallbacks>(
423424
&Compiler, OverlayFS);
424425
ModuleExpanderPP = ModuleExpander->getPreprocessor();

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,11 @@ ClangTidyError::ClangTidyError(StringRef CheckName,
161161

162162
ClangTidyContext::ClangTidyContext(
163163
std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
164-
bool AllowEnablingAnalyzerAlphaCheckers)
164+
bool AllowEnablingAnalyzerAlphaCheckers, bool EnableModuleHeadersParsing)
165165
: DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)),
166166
Profile(false),
167167
AllowEnablingAnalyzerAlphaCheckers(AllowEnablingAnalyzerAlphaCheckers),
168+
EnableModuleHeadersParsing(EnableModuleHeadersParsing),
168169
SelfContainedDiags(false) {
169170
// Before the first translation unit we can get errors related to command-line
170171
// parsing, use empty string for the file name in this case.

clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h

+9-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ class ClangTidyContext {
7070
public:
7171
/// Initializes \c ClangTidyContext instance.
7272
ClangTidyContext(std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider,
73-
bool AllowEnablingAnalyzerAlphaCheckers = false);
73+
bool AllowEnablingAnalyzerAlphaCheckers = false,
74+
bool EnableModuleHeadersParsing = false);
7475
/// Sets the DiagnosticsEngine that diag() will emit diagnostics to.
7576
// FIXME: this is required initialization, and should be a constructor param.
7677
// Fix the context -> diag engine -> consumer -> context initialization cycle.
@@ -198,6 +199,12 @@ class ClangTidyContext {
198199
return AllowEnablingAnalyzerAlphaCheckers;
199200
}
200201

202+
// This method determines whether preprocessor-level module header parsing is
203+
// enabled using the `--experimental-enable-module-headers-parsing` option.
204+
bool canEnableModuleHeadersParsing() const {
205+
return EnableModuleHeadersParsing;
206+
}
207+
201208
void setSelfContainedDiags(bool Value) { SelfContainedDiags = Value; }
202209

203210
bool areDiagsSelfContained() const { return SelfContainedDiags; }
@@ -245,6 +252,7 @@ class ClangTidyContext {
245252
std::string ProfilePrefix;
246253

247254
bool AllowEnablingAnalyzerAlphaCheckers;
255+
bool EnableModuleHeadersParsing;
248256

249257
bool SelfContainedDiags;
250258

clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp

+13-1
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,17 @@ static cl::opt<bool>
263263
cl::init(false), cl::Hidden,
264264
cl::cat(ClangTidyCategory));
265265

266+
static cl::opt<bool> EnableModuleHeadersParsing("enable-module-headers-parsing",
267+
desc(R"(
268+
Enables preprocessor-level module header parsing
269+
for C++20 and above, empowering specific checks
270+
to detect macro definitions within modules. This
271+
feature may cause performance and parsing issues
272+
and is therefore considered experimental.
273+
)"),
274+
cl::init(false),
275+
cl::cat(ClangTidyCategory));
276+
266277
static cl::opt<std::string> ExportFixes("export-fixes", desc(R"(
267278
YAML file to store suggested fixes in. The
268279
stored fixes can be applied to the input source
@@ -659,7 +670,8 @@ int clangTidyMain(int argc, const char **argv) {
659670
llvm::InitializeAllAsmParsers();
660671

661672
ClangTidyContext Context(std::move(OwningOptionsProvider),
662-
AllowEnablingAnalyzerAlphaCheckers);
673+
AllowEnablingAnalyzerAlphaCheckers,
674+
EnableModuleHeadersParsing);
663675
std::vector<ClangTidyError> Errors =
664676
runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
665677
FixNotes, EnableCheckProfile, ProfilePrefix);

clang-tools-extra/docs/ReleaseNotes.rst

+14
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,20 @@ The improvements are...
9494
Improvements to clang-tidy
9595
--------------------------
9696

97+
- Preprocessor-level module header parsing is now disabled by default due to
98+
the problems it caused in C++20 and above, leading to performance and code
99+
parsing issues regardless of whether modules were used or not. This change
100+
will impact only the following checks:
101+
:doc:`modernize-replace-disallow-copy-and-assign-macro
102+
<clang-tidy/checks/modernize/replace-disallow-copy-and-assign-macro>`,
103+
:doc:`bugprone-reserved-identifier
104+
<clang-tidy/checks/bugprone/reserved-identifier>`, and
105+
:doc:`readability-identifier-naming
106+
<clang-tidy/checks/readability/identifier-naming>`. Those checks will no
107+
longer see macros defined in modules. Users can still enable this
108+
functionality using the newly added command line option
109+
`--enable-module-headers-parsing`.
110+
97111
New checks
98112
^^^^^^^^^^
99113

0 commit comments

Comments
 (0)