Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][lex] Always pass suggested module to InclusionDirective() callback #81061

Merged
merged 4 commits into from
Feb 8, 2024

Conversation

jansvoboda11
Copy link
Contributor

@jansvoboda11 jansvoboda11 commented Feb 8, 2024

This patch provides more information to the PPCallbacks::InclusionDirective() hook. We now always pass the suggested module, regardless of whether it was actually imported or not. The extra bool ModuleImported parameter then denotes whether the header #include will be automatically translated into import the the module.

The main change is in clang/lib/Lex/PPDirectives.cpp, where we take care to not modify SuggestedModule after it's been populated by LookupHeaderIncludeOrImport(). We now exclusively use the SM (ModuleToImport) variable instead, which has been equivalent to SuggestedModule until now. This allows us to use the original non-modified SuggestedModule for the callback itself.

(This patch turns out to be necessary for apple#8011).

@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tools-extra clangd clang-tidy clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang:codegen labels Feb 8, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 8, 2024

@llvm/pr-subscribers-clang-modules
@llvm/pr-subscribers-clang-codegen
@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

Changes

This change gives clients more context and turns out to be necessary in apple#8011.

The main change is in clang/lib/Lex/PPDirectives.cpp, where we take care to not modify SuggestedModule after it's been populated by LookupHeaderIncludeOrImport(). We now exclusively use the SM variable instead, which has been equivalent to SuggestedModule until now. This allows us to use the original non-modified SuggestedModule for the callback itself.


Patch is 52.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/81061.diff

40 Files Affected:

  • (modified) clang-tools-extra/clang-move/Move.cpp (+2-1)
  • (modified) clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp (+3-3)
  • (modified) clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h (+1-1)
  • (modified) clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp (+3-2)
  • (modified) clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp (+4-3)
  • (modified) clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp (+4-3)
  • (modified) clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp (+5-4)
  • (modified) clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp (+1-1)
  • (modified) clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp (+4-3)
  • (modified) clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp (+2-1)
  • (modified) clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp (+2-2)
  • (modified) clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h (+2-1)
  • (modified) clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp (+4-3)
  • (modified) clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp (+2-1)
  • (modified) clang-tools-extra/clangd/Headers.cpp (+2-1)
  • (modified) clang-tools-extra/clangd/ParsedAST.cpp (+1-1)
  • (modified) clang-tools-extra/clangd/index/IndexAction.cpp (+2-1)
  • (modified) clang-tools-extra/clangd/unittests/ReplayPeambleTests.cpp (+1-1)
  • (modified) clang-tools-extra/include-cleaner/lib/Record.cpp (+4-2)
  • (modified) clang-tools-extra/modularize/CoverageChecker.cpp (+2-1)
  • (modified) clang-tools-extra/modularize/PreprocessorTracker.cpp (+10-10)
  • (modified) clang-tools-extra/pp-trace/PPCallbacksTracker.cpp (+4-2)
  • (modified) clang-tools-extra/pp-trace/PPCallbacksTracker.h (+2-1)
  • (modified) clang-tools-extra/test/pp-trace/pp-trace-include.cpp (+8-4)
  • (modified) clang/include/clang/Lex/PPCallbacks.h (+12-6)
  • (modified) clang/include/clang/Lex/PreprocessingRecord.h (+2-1)
  • (modified) clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h (+2-1)
  • (modified) clang/lib/CodeGen/MacroPPCallbacks.cpp (+2-2)
  • (modified) clang/lib/CodeGen/MacroPPCallbacks.h (+2-1)
  • (modified) clang/lib/Frontend/DependencyFile.cpp (+2-1)
  • (modified) clang/lib/Frontend/DependencyGraph.cpp (+4-3)
  • (modified) clang/lib/Frontend/ModuleDependencyCollector.cpp (+2-1)
  • (modified) clang/lib/Frontend/PrecompiledPreamble.cpp (+2-1)
  • (modified) clang/lib/Frontend/PrintPreprocessedOutput.cpp (+6-5)
  • (modified) clang/lib/Frontend/Rewrite/InclusionRewriter.cpp (+6-4)
  • (modified) clang/lib/Lex/PPDirectives.cpp (+8-12)
  • (modified) clang/lib/Lex/PreprocessingRecord.cpp (+5-6)
  • (modified) clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp (+4-4)
  • (modified) clang/tools/libclang/Indexing.cpp (+3-2)
  • (modified) clang/unittests/Lex/PPCallbacksTest.cpp (+6-3)
diff --git a/clang-tools-extra/clang-move/Move.cpp b/clang-tools-extra/clang-move/Move.cpp
index 1d10348430c281..ac16803b46783e 100644
--- a/clang-tools-extra/clang-move/Move.cpp
+++ b/clang-tools-extra/clang-move/Move.cpp
@@ -133,7 +133,8 @@ class FindAllIncludes : public PPCallbacks {
                           CharSourceRange FilenameRange,
                           OptionalFileEntryRef /*File*/, StringRef SearchPath,
                           StringRef /*RelativePath*/,
-                          const Module * /*Imported*/,
+                          const Module * /*SuggestedModule*/,
+                          bool /*ModuleImported*/,
                           SrcMgr::CharacteristicKind /*FileType*/) override {
     if (auto FileEntry = SM.getFileEntryRefForID(SM.getFileID(HashLoc)))
       MoveTool->addIncludes(FileName, IsAngled, SearchPath,
diff --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
index 5ecd4fb19131e4..5e2cc207560d33 100644
--- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
+++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
@@ -166,12 +166,12 @@ void ExpandModularHeadersPPCallbacks::InclusionDirective(
     SourceLocation DirectiveLoc, const Token &IncludeToken,
     StringRef IncludedFilename, bool IsAngled, CharSourceRange FilenameRange,
     OptionalFileEntryRef IncludedFile, StringRef SearchPath,
-    StringRef RelativePath, const Module *Imported,
+    StringRef RelativePath, const Module *SuggestedModule, bool ModuleImported,
     SrcMgr::CharacteristicKind FileType) {
-  if (Imported) {
+  if (ModuleImported) {
     serialization::ModuleFile *MF =
         Compiler.getASTReader()->getModuleManager().lookup(
-            *Imported->getASTFile());
+            *SuggestedModule->getASTFile());
     handleModuleFile(MF);
   }
   parseToLocation(DirectiveLoc);
diff --git a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h
index 3f6abc315e5b90..0742c21bc43720 100644
--- a/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h
+++ b/clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h
@@ -69,7 +69,7 @@ class ExpandModularHeadersPPCallbacks : public PPCallbacks {
                           bool IsAngled, CharSourceRange FilenameRange,
                           OptionalFileEntryRef IncludedFile,
                           StringRef SearchPath, StringRef RelativePath,
-                          const Module *Imported,
+                          const Module *SuggestedModule, bool ModuleImported,
                           SrcMgr::CharacteristicKind FileType) override;
 
   void EndOfMainFile() override;
diff --git a/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp b/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
index 084e44a714d1ff..fb1e0e82a3149b 100644
--- a/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp
@@ -29,7 +29,8 @@ class KernelNameRestrictionPPCallbacks : public PPCallbacks {
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FileNameRange,
                           OptionalFileEntryRef File, StringRef SearchPath,
-                          StringRef RelativePath, const Module *Imported,
+                          StringRef RelativePath, const Module *SuggestedModule,
+                          bool ModuleImported,
                           SrcMgr::CharacteristicKind FileType) override;
 
   void EndOfMainFile() override;
@@ -61,7 +62,7 @@ void KernelNameRestrictionCheck::registerPPCallbacks(const SourceManager &SM,
 void KernelNameRestrictionPPCallbacks::InclusionDirective(
     SourceLocation HashLoc, const Token &, StringRef FileName, bool,
     CharSourceRange, OptionalFileEntryRef, StringRef, StringRef, const Module *,
-    SrcMgr::CharacteristicKind) {
+    bool, SrcMgr::CharacteristicKind) {
   IncludeDirective ID = {HashLoc, FileName};
   IncludeDirectives.push_back(std::move(ID));
 }
diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
index 61d89cf3081306..09ba79f0557525 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
@@ -26,7 +26,8 @@ class SuspiciousIncludePPCallbacks : public PPCallbacks {
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange,
                           OptionalFileEntryRef File, StringRef SearchPath,
-                          StringRef RelativePath, const Module *Imported,
+                          StringRef RelativePath, const Module *SuggestedModule,
+                          bool ModuleImported,
                           SrcMgr::CharacteristicKind FileType) override;
 
 private:
@@ -51,8 +52,8 @@ void SuspiciousIncludeCheck::registerPPCallbacks(
 void SuspiciousIncludePPCallbacks::InclusionDirective(
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
     bool IsAngled, CharSourceRange FilenameRange, OptionalFileEntryRef File,
-    StringRef SearchPath, StringRef RelativePath, const Module *Imported,
-    SrcMgr::CharacteristicKind FileType) {
+    StringRef SearchPath, StringRef RelativePath, const Module *SuggestedModule,
+    bool ModuleImported, SrcMgr::CharacteristicKind FileType) {
   if (IncludeTok.getIdentifierInfo()->getPPKeywordID() == tok::pp_import)
     return;
 
diff --git a/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp b/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
index bdd72f85e2a27c..4246c8c574c50d 100644
--- a/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp
@@ -27,7 +27,8 @@ class IncludeOrderPPCallbacks : public PPCallbacks {
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange,
                           OptionalFileEntryRef File, StringRef SearchPath,
-                          StringRef RelativePath, const Module *Imported,
+                          StringRef RelativePath, const Module *SuggestedModule,
+                          bool ModuleImported,
                           SrcMgr::CharacteristicKind FileType) override;
   void EndOfMainFile() override;
 
@@ -81,8 +82,8 @@ static int getPriority(StringRef Filename, bool IsAngled, bool IsMainModule) {
 void IncludeOrderPPCallbacks::InclusionDirective(
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
     bool IsAngled, CharSourceRange FilenameRange, OptionalFileEntryRef File,
-    StringRef SearchPath, StringRef RelativePath, const Module *Imported,
-    SrcMgr::CharacteristicKind FileType) {
+    StringRef SearchPath, StringRef RelativePath, const Module *SuggestedModule,
+    bool ModuleImported, SrcMgr::CharacteristicKind FileType) {
   // We recognize the first include as a special main module header and want
   // to leave it in the top position.
   IncludeDirective ID = {HashLoc, FilenameRange, std::string(FileName),
diff --git a/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp b/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp
index 3451d3474fd906..b656917071a6ca 100644
--- a/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp
@@ -33,7 +33,8 @@ class RestrictedIncludesPPCallbacks
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange,
                           OptionalFileEntryRef File, StringRef SearchPath,
-                          StringRef RelativePath, const Module *Imported,
+                          StringRef RelativePath, const Module *SuggestedModule,
+                          bool ModuleImported,
                           SrcMgr::CharacteristicKind FileType) override;
 
 private:
@@ -45,14 +46,14 @@ class RestrictedIncludesPPCallbacks
 void RestrictedIncludesPPCallbacks::InclusionDirective(
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
     bool IsAngled, CharSourceRange FilenameRange, OptionalFileEntryRef File,
-    StringRef SearchPath, StringRef RelativePath, const Module *Imported,
-    SrcMgr::CharacteristicKind FileType) {
+    StringRef SearchPath, StringRef RelativePath, const Module *SuggestedModule,
+    bool ModuleImported, SrcMgr::CharacteristicKind FileType) {
   // Compiler provided headers are allowed (e.g stddef.h).
   if (SrcMgr::isSystem(FileType) && SearchPath == CompilerIncudeDir)
     return;
   portability::RestrictedIncludesPPCallbacks::InclusionDirective(
       HashLoc, IncludeTok, FileName, IsAngled, FilenameRange, File, SearchPath,
-      RelativePath, Imported, FileType);
+      RelativePath, SuggestedModule, ModuleImported, FileType);
 }
 
 void RestrictSystemLibcHeadersCheck::registerPPCallbacks(
diff --git a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
index bebd6e390ed53c..fadfdc869d37b0 100644
--- a/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp
@@ -83,7 +83,7 @@ class CyclicDependencyCallbacks : public PPCallbacks {
   void InclusionDirective(SourceLocation, const Token &, StringRef FilePath,
                           bool, CharSourceRange Range,
                           OptionalFileEntryRef File, StringRef, StringRef,
-                          const Module *,
+                          const Module *, bool,
                           SrcMgr::CharacteristicKind FileType) override {
     if (FileType != clang::SrcMgr::C_User)
       return;
diff --git a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
index 030a781e2099be..6d287eb3642dfa 100644
--- a/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp
@@ -32,7 +32,8 @@ class IncludeModernizePPCallbacks : public PPCallbacks {
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange,
                           OptionalFileEntryRef File, StringRef SearchPath,
-                          StringRef RelativePath, const Module *Imported,
+                          StringRef RelativePath, const Module *SuggestedModule,
+                          bool ModuleImported,
                           SrcMgr::CharacteristicKind FileType) override;
 
 private:
@@ -178,8 +179,8 @@ IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(
 void IncludeModernizePPCallbacks::InclusionDirective(
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
     bool IsAngled, CharSourceRange FilenameRange, OptionalFileEntryRef File,
-    StringRef SearchPath, StringRef RelativePath, const Module *Imported,
-    SrcMgr::CharacteristicKind FileType) {
+    StringRef SearchPath, StringRef RelativePath, const Module *SuggestedModule,
+    bool ModuleImported, SrcMgr::CharacteristicKind FileType) {
 
   // If we don't want to warn for non-main file reports and this is one, skip
   // it.
diff --git a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
index b197c22dca410e..0b47ed316ca271 100644
--- a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
@@ -117,7 +117,8 @@ class MacroToEnumCallbacks : public PPCallbacks {
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange,
                           OptionalFileEntryRef File, StringRef SearchPath,
-                          StringRef RelativePath, const Module *Imported,
+                          StringRef RelativePath, const Module *SuggestedModule,
+                          bool ModuleImported,
                           SrcMgr::CharacteristicKind FileType) override {
     clearCurrentEnum(HashLoc);
   }
diff --git a/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp b/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp
index 9ee0b4e6d3ccb8..db5693e3b7cb7d 100644
--- a/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp
@@ -21,8 +21,8 @@ namespace clang::tidy::portability {
 void RestrictedIncludesPPCallbacks::InclusionDirective(
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
     bool IsAngled, CharSourceRange FilenameRange, OptionalFileEntryRef File,
-    StringRef SearchPath, StringRef RelativePath, const Module *Imported,
-    SrcMgr::CharacteristicKind FileType) {
+    StringRef SearchPath, StringRef RelativePath, const Module *SuggestedModule,
+    bool ModuleImported, SrcMgr::CharacteristicKind FileType) {
   if (!Check.contains(FileName) && SrcMgr::isSystem(FileType)) {
     SmallString<256> FullPath;
     llvm::sys::path::append(FullPath, SearchPath);
diff --git a/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h b/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h
index ad18e6f411dbbd..60fae5e73a6026 100644
--- a/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h
+++ b/clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h
@@ -50,7 +50,8 @@ class RestrictedIncludesPPCallbacks : public PPCallbacks {
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange,
                           OptionalFileEntryRef File, StringRef SearchPath,
-                          StringRef RelativePath, const Module *Imported,
+                          StringRef RelativePath, const Module *SuggestedModule,
+                          bool ModuleImported,
                           SrcMgr::CharacteristicKind FileType) override;
   void EndOfMainFile() override;
 
diff --git a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
index d1f41e0ec79e21..67147164946ab4 100644
--- a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
@@ -47,7 +47,8 @@ class DuplicateIncludeCallbacks : public PPCallbacks {
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange,
                           OptionalFileEntryRef File, StringRef SearchPath,
-                          StringRef RelativePath, const Module *Imported,
+                          StringRef RelativePath, const Module *SuggestedModule,
+                          bool ModuleImported,
                           SrcMgr::CharacteristicKind FileType) override;
 
   void MacroDefined(const Token &MacroNameTok,
@@ -76,8 +77,8 @@ void DuplicateIncludeCallbacks::FileChanged(SourceLocation Loc,
 void DuplicateIncludeCallbacks::InclusionDirective(
     SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
     bool IsAngled, CharSourceRange FilenameRange, OptionalFileEntryRef File,
-    StringRef SearchPath, StringRef RelativePath, const Module *Imported,
-    SrcMgr::CharacteristicKind FileType) {
+    StringRef SearchPath, StringRef RelativePath, const Module *SuggestedModule,
+    bool ModuleImported, SrcMgr::CharacteristicKind FileType) {
   if (llvm::is_contained(Files.back(), FileName)) {
     // We want to delete the entire line, so make sure that [Start,End] covers
     // everything.
diff --git a/clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp b/clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp
index d0b7474992abd0..b53016f331b793 100644
--- a/clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp
+++ b/clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp
@@ -25,7 +25,8 @@ class IncludeInserterCallback : public PPCallbacks {
                           bool IsAngled, CharSourceRange FileNameRange,
                           OptionalFileEntryRef /*IncludedFile*/,
                           StringRef /*SearchPath*/, StringRef /*RelativePath*/,
-                          const Module * /*ImportedModule*/,
+                          const Module * /*SuggestedModule*/,
+                          bool /*ModuleImported*/,
                           SrcMgr::CharacteristicKind /*FileType*/) override {
     Inserter->addInclude(FileNameRef, IsAngled, HashLocation,
                          IncludeToken.getEndLoc());
diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp
index 076e636e0e2819..75f8668e7bef06 100644
--- a/clang-tools-extra/clangd/Headers.cpp
+++ b/clang-tools-extra/clangd/Headers.cpp
@@ -41,7 +41,8 @@ class IncludeStructure::RecordHeaders : public PPCallbacks {
                           OptionalFileEntryRef File,
                           llvm::StringRef /*SearchPath*/,
                           llvm::StringRef /*RelativePath*/,
-                          const clang::Module * /*Imported*/,
+                          const clang::Module * /*SuggestedModule*/,
+                          bool /*ModuleImported*/,
                           SrcMgr::CharacteristicKind FileKind) override {
     auto MainFID = SM.getMainFileID();
     // If an include is part of the preamble patch, translate #line directives.
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
index 14a91797f4d2ea..bbb0e2c77b3f31 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -244,7 +244,7 @@ class ReplayPreamble : private PPCallbacks {
                             SynthesizedFilenameTok.getEndLoc())
               .toCharRange(SM),
           File, "SearchPath", "RelPath",
-          /*Imported=*/nullptr, Inc.FileKind);
+          /*SuggestedModule=*/nullptr, /*ModuleImported=*/false, Inc.FileKind);
       if (File)
         Delegate->FileSkipped(*File, SynthesizedFilenameTok, Inc.FileKind);
     }
diff --git a/clang-tools-extra/clangd/index/IndexAction.cpp b/clang-tools-extra/clangd/index/IndexAction.cpp
index 5d56285a839614..ed56c2a9d2e811 100644
--- a/clang-tools-extra/clangd/index/IndexAction.cpp
+++ b/clang-tools-extra/clangd/index/IndexAction.cpp
@@ -89,7 +89,8 @@ struct IncludeGraphCollector : public PPCallbacks {
                           llvm::StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange,
                           OptionalFileEntryRef File, llvm::StringRef SearchPath,
-                          llvm::StringRef RelativePath, const Module *Imported,
+                          llvm::StringRef RelativePath,
+                          const Module *SuggestedModule, bool ModuleImported,
                           SrcMgr::CharacteristicKind FileType) override {
     auto IncludeURI = toURI(File);
     if (!IncludeURI)
diff --git a/clang-tools-extra/clangd/unittests/ReplayPeambleTests.cpp b/clang-tools-extra/clangd/unittests/ReplayPeambleTests.cpp
index 472fe30ee46ed4..147d9abe691372 100644
--- a/clang-tools-extra/clangd/unittests/ReplayPeambleTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ReplayPeambleTests.cpp
@@ -72,7 +72,7 @@ struct ReplayPreamblePPCallback : public PPCallbacks {
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
                           StringRef FileName, bool IsAngled,
                           CharSourceRange FilenameRange, OptionalFileEntryRef,
-                          StringRef, StringRef, const clang::Module *,
+                          StringRef, StringRef, const clang::Module *, bool,
                           SrcMgr::CharacteristicKind) override {
     Includes.emplace_back(SM, HashLoc, IncludeTok, FileName, IsAngled,
                           FilenameRange);
diff --git a/clang-tools-extra/include-clea...
[truncated]

Copy link

github-actions bot commented Feb 8, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff e71a5f54d86be3ddf66d4a4e53d5083ef7f7a118 1a80d25f147087d596dfacce33afc8064f44040d -- clang-tools-extra/clang-move/Move.cpp clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp clang-tools-extra/clang-tidy/llvm/IncludeOrderCheck.cpp clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp clang-tools-extra/clang-tidy/misc/HeaderIncludeCycleCheck.cpp clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.cpp clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp clang-tools-extra/clangd/Headers.cpp clang-tools-extra/clangd/ParsedAST.cpp clang-tools-extra/clangd/index/IndexAction.cpp clang-tools-extra/clangd/unittests/ReplayPeambleTests.cpp clang-tools-extra/include-cleaner/lib/Record.cpp clang-tools-extra/modularize/CoverageChecker.cpp clang-tools-extra/modularize/PreprocessorTracker.cpp clang-tools-extra/pp-trace/PPCallbacksTracker.cpp clang-tools-extra/pp-trace/PPCallbacksTracker.h clang-tools-extra/test/pp-trace/pp-trace-include.cpp clang/include/clang/Lex/PPCallbacks.h clang/include/clang/Lex/PreprocessingRecord.h clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h clang/lib/CodeGen/MacroPPCallbacks.cpp clang/lib/CodeGen/MacroPPCallbacks.h clang/lib/Frontend/DependencyFile.cpp clang/lib/Frontend/DependencyGraph.cpp clang/lib/Frontend/ModuleDependencyCollector.cpp clang/lib/Frontend/PrecompiledPreamble.cpp clang/lib/Frontend/PrintPreprocessedOutput.cpp clang/lib/Frontend/Rewrite/InclusionRewriter.cpp clang/lib/Lex/PPDirectives.cpp clang/lib/Lex/PreprocessingRecord.cpp clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp clang/tools/libclang/Indexing.cpp clang/unittests/Lex/PPCallbacksTest.cpp
View the diff from clang-format here.
diff --git a/clang/lib/Frontend/PrintPreprocessedOutput.cpp b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
index a26d2c3ab8..4f838362b7 100644
--- a/clang/lib/Frontend/PrintPreprocessedOutput.cpp
+++ b/clang/lib/Frontend/PrintPreprocessedOutput.cpp
@@ -428,9 +428,8 @@ void PrintPPOutputPPCallbacks::InclusionDirective(
       *OS << "#pragma clang module import "
           << SuggestedModule->getFullModuleName(true)
           << " /* clang -E: implicit import for "
-          << "#" << PP.getSpelling(IncludeTok) << " "
-          << (IsAngled ? '<' : '"') << FileName << (IsAngled ? '>' : '"')
-          << " */";
+          << "#" << PP.getSpelling(IncludeTok) << " " << (IsAngled ? '<' : '"')
+          << FileName << (IsAngled ? '>' : '"') << " */";
       setEmittedDirectiveOnThisLine();
       break;
 

Copy link
Collaborator

@benlangmuir benlangmuir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to update your commit message to talk about the benefits of this change: the callback now provides more information about the included header and models the associated module accurately regardless of whether it is translated to an import.

clang/include/clang/Lex/PPCallbacks.h Outdated Show resolved Hide resolved
clang/lib/Lex/PPDirectives.cpp Outdated Show resolved Hide resolved
@jansvoboda11 jansvoboda11 merged commit da95d92 into llvm:main Feb 8, 2024
2 of 6 checks passed
@jansvoboda11 jansvoboda11 deleted the pp-cb-suggested-module branch February 8, 2024 18:19
jansvoboda11 added a commit to apple/llvm-project that referenced this pull request Feb 8, 2024
…allback (llvm#81061)

This patch provides more information to the
`PPCallbacks::InclusionDirective()` hook. We now always pass the
suggested module, regardless of whether it was actually imported or not.
The extra `bool ModuleImported` parameter then denotes whether the
header `#include` will be automatically translated into import the the
module.

The main change is in `clang/lib/Lex/PPDirectives.cpp`, where we take
care to not modify `SuggestedModule` after it's been populated by
`LookupHeaderIncludeOrImport()`. We now exclusively use the `SM`
(`ModuleToImport`) variable instead, which has been equivalent to
`SuggestedModule` until now. This allows us to use the original
non-modified `SuggestedModule` for the callback itself.

(This patch turns out to be necessary for
#8011).

(cherry picked from commit da95d92)
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Feb 12, 2024
…allback (llvm#81061)

This patch provides more information to the
`PPCallbacks::InclusionDirective()` hook. We now always pass the
suggested module, regardless of whether it was actually imported or not.
The extra `bool ModuleImported` parameter then denotes whether the
header `#include` will be automatically translated into import the the
module.

The main change is in `clang/lib/Lex/PPDirectives.cpp`, where we take
care to not modify `SuggestedModule` after it's been populated by
`LookupHeaderIncludeOrImport()`. We now exclusively use the `SM`
(`ModuleToImport`) variable instead, which has been equivalent to
`SuggestedModule` until now. This allows us to use the original
non-modified `SuggestedModule` for the callback itself.

(This patch turns out to be necessary for
apple#8011).

Change-Id: I6f64614c4468ae4737b20609e065d69390d2e443
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:codegen clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category clang-tidy clang-tools-extra clangd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants