[clang-tidy] Add --enable-module-headers-parsing option

[PiotrZSL]

Summary

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.

[carlosgalvezp]

Please squash into previous patch, I see no reason to make them into separate commits. The first one is missing Release Notes, for example.

[PiotrZSL]

@carlosgalvezp
My plan was to push first change regardless (so they would be some workaround, even if hidden & undocumented), and second only if approved.

[PiotrZSL]

Merge with other change, remove experimental

[carlosgalvezp]

Do we want to keep the experimental word in the flag?

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

266

--experimental-enable-module-headers-parsing

270

Should we document the implications/risks of enabling this, so people are informed? Also the fact that is experimental and subject to change.

clang-tools-extra/docs/ReleaseNotes.rst

109

experimental

[PiotrZSL]

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

266

I removed 'experimental' because it were messing with the --help output, were causing it be very wide.
And it's no longer hidden option now. so should it be hidden & experimental ? Or non hidden...

270

Something like "May cause performance & parsing issues, and therefore is considered experimental." ? I'ts fine with me.

[carlosgalvezp]

LGTM! Feel free to add the comment about the implications of using the flag in the docs.

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

270

Sounds good to me!

[PiotrZSL]

Fix doc

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

ClangTidyASTConsumerFactory::createASTConsumer(
   }
 
   std::unique_ptr<ast_matchers::MatchFinder> Finder(
       new ast_matchers::MatchFinder(std::move(FinderOptions)));
 
   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();
     PP->addPPCallbacks(std::move(ModuleExpander));
   }
 
   for (auto &Check : Checks) {
     Check->registerMatchers(&*Finder);

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

 /// \code
 /// Context->Diag(Loc, "Single-argument constructors must be explicit")
 ///     << FixItHint::CreateInsertion(Loc, "explicit ");
 /// \endcode
 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.
   void setDiagnosticsEngine(DiagnosticsEngine *DiagEngine) {
     this->DiagEngine = DiagEngine;
   }
 
   ~ClangTidyContext();
public:
   }
 
   /// If the experimental alpha checkers from the static analyzer can be
   /// enabled.
   bool canEnableAnalyzerAlphaCheckers() const {
     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; }
 
   using DiagLevelAndFormatString = std::pair<DiagnosticIDs::Level, std::string>;
   DiagLevelAndFormatString getDiagLevelAndFormatString(unsigned DiagnosticID,
                                                        SourceLocation Loc) {
     return DiagLevelAndFormatString(
private:
   std::string CurrentBuildDirectory;
 
   llvm::DenseMap<unsigned, std::string> CheckNamesByDiagnosticID;
 
   bool Profile;
   std::string ProfilePrefix;
 
   bool AllowEnablingAnalyzerAlphaCheckers;
+  bool EnableModuleHeadersParsing;
 
   bool SelfContainedDiags;
 
   NoLintDirectiveHandler NoLintHandler;
   llvm::StringSet<> *OptionsCollector = nullptr;
 };
 
 /// Gets the Fix attached to \p Diagnostic.

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

 ClangTidyError::ClangTidyError(StringRef CheckName,
                                ClangTidyError::Level DiagLevel,
                                StringRef BuildDirectory, bool IsWarningAsError)
     : tooling::Diagnostic(CheckName, DiagLevel, BuildDirectory),
       IsWarningAsError(IsWarningAsError) {}
 
 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.
   setCurrentFile("");
 }
 
 ClangTidyContext::~ClangTidyContext() = default;
 

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

 /// This option allows enabling the experimental alpha checkers from the static
 /// analyzer. This option is set to false and not visible in help, because it is
 /// highly not recommended for users.
 static cl::opt<bool>
     AllowEnablingAnalyzerAlphaCheckers("allow-enabling-analyzer-alpha-checkers",
                                        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
 code with clang-apply-replacements.
 )"),
                                         cl::value_desc("filename"),
                                         cl::cat(ClangTidyCategory));
 
if (PathList.empty()) {
     return 1;
   }
 
   llvm::InitializeAllTargetInfos();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
 
   ClangTidyContext Context(std::move(OwningOptionsProvider),
-                           AllowEnablingAnalyzerAlphaCheckers);
+                           AllowEnablingAnalyzerAlphaCheckers,
+                           EnableModuleHeadersParsing);
   std::vector<ClangTidyError> Errors =
       runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
                    FixNotes, EnableCheckProfile, ProfilePrefix);
   bool FoundErrors = llvm::any_of(Errors, [](const ClangTidyError &E) {
     return E.DiagLevel == ClangTidyError::Error;
   });
 
   // --fix-errors and --fix-notes imply --fix.

clang-tools-extra/docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 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
 ^^^^^^^^^^
 
 - New :doc:`performance-enum-size
   <clang-tidy/checks/performance/enum-size>` check.
 
   Recommends the smallest possible underlying type for an ``enum`` or ``enum``
   class based on the range of its enumerators.

clang-tools-extra/docs/clang-tidy/index.rst

.. code-block:: console
 
   $ clang-tidy --help
   USAGE: clang-tidy [options] <source0> [... <sourceN>]
 
   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 '-'
+    --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=<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:
+    --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
+    --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
+    --enable-check-profile          - Enable per-check timing profiles, and print a
                                       report to stderr.
-    --explain-config               - For each enabled check explains, where it is
+    --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
+    --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
+    --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
+    --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
+    --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:
+    --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
+    --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
+    --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
+    --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
+    --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
+    --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.
+    --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
+    --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
+    --verify-config                 - Check the config files to ensure each check and
                                       option is recognized.
-    --vfsoverlay=<filename>        - Overlay the virtual filesystem described by file
+    --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
+    --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.
 
     For example, it can be a CMake build directory in which a file named
     compile_commands.json exists (use -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
     CMake option to get this output). When no build path is specified,
     a search for compile_commands.json will be attempted through all
     parent paths of the first input file . See:

clang-tools-extra/test/clang-tidy/infrastructure/expand-modular-headers-ppcallbacks.cpp

 // Sanity-check. Run without modules:
 // RUN: rm -rf %t
 // RUN: mkdir %t
 // RUN: cp %S/Inputs/expand-modular-headers-ppcallbacks/* %t/
 // 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
 // RUN: mkdir %t
 // RUN: cp %S/Inputs/expand-modular-headers-ppcallbacks/* %t/
 // 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:
 //
 // RUN: rm -rf %t
 // RUN: mkdir %t
 // RUN: cp %S/Inputs/expand-modular-headers-ppcallbacks/* %t/
 // 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 \
 // RUN:   -fmodules-cache-path=%t/module-cache/
 //
 // RUN: rm -rf %t
 // RUN: mkdir %t
 // RUN: cp %S/Inputs/expand-modular-headers-ppcallbacks/* %t/
 // 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 \
 // RUN:   -fmodules-cache-path=%t/module-cache/
 // FIXME: Make the test work in all language modes.
 #include "c.h"
 
 // CHECK-MESSAGES: a.h:1:9: warning: invalid case style for macro definition 'a' [readability-identifier-naming]