[clang] 807aa26 - [C++20] [Modules] Don't ignore -fmodule-file when we compile pcm files

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Mon May 22 23:22:23 PDT 2023


Author: Chuanqi Xu
Date: 2023-05-23T14:22:01+08:00
New Revision: 807aa261361f4cd9153dda43c9cadbd3fcc659c5

URL: https://github.com/llvm/llvm-project/commit/807aa261361f4cd9153dda43c9cadbd3fcc659c5
DIFF: https://github.com/llvm/llvm-project/commit/807aa261361f4cd9153dda43c9cadbd3fcc659c5.diff

LOG: [C++20] [Modules] Don't ignore -fmodule-file when we compile pcm files

Close https://github.com/llvm/llvm-project/issues/62843.

Previously when we compile .pcm files into .o files, the
`-fmodule-file=<module-name>=<module-path>` option is ignored. This is
conflicted with our consensus in
https://github.com/llvm/llvm-project/issues/62707.

Added: 
    

Modified: 
    clang/include/clang/Frontend/ASTUnit.h
    clang/lib/CrossTU/CrossTranslationUnit.cpp
    clang/lib/Driver/ToolChains/Clang.cpp
    clang/lib/Frontend/ASTMerge.cpp
    clang/lib/Frontend/ASTUnit.cpp
    clang/lib/Frontend/FrontendAction.cpp
    clang/test/Modules/no-implicit-std-cxx-module.cppm
    clang/tools/c-index-test/core_main.cpp
    clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
    clang/tools/libclang/CIndex.cpp
    clang/unittests/Frontend/ASTUnitTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Frontend/ASTUnit.h b/clang/include/clang/Frontend/ASTUnit.h
index cb1ea39c17f86..05d149ee6dff8 100644
--- a/clang/include/clang/Frontend/ASTUnit.h
+++ b/clang/include/clang/Frontend/ASTUnit.h
@@ -694,6 +694,7 @@ class ASTUnit {
                   const PCHContainerReader &PCHContainerRdr, WhatToLoad ToLoad,
                   IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
                   const FileSystemOptions &FileSystemOpts,
+                  std::shared_ptr<HeaderSearchOptions> HSOpts,
                   bool UseDebugInfo = false, bool OnlyLocalDecls = false,
                   CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::None,
                   bool AllowASTWithCompilerErrors = false,

diff  --git a/clang/lib/CrossTU/CrossTranslationUnit.cpp b/clang/lib/CrossTU/CrossTranslationUnit.cpp
index e84ef0a590e71..ad4657ddcedea 100644
--- a/clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ b/clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -568,7 +568,7 @@ CrossTranslationUnitContext::ASTLoader::loadFromDump(StringRef ASTDumpPath) {
   return ASTUnit::LoadFromASTFile(
       std::string(ASTDumpPath.str()),
       CI.getPCHContainerOperations()->getRawReader(), ASTUnit::LoadEverything,
-      Diags, CI.getFileSystemOpts());
+      Diags, CI.getFileSystemOpts(), CI.getHeaderSearchOptsPtr());
 }
 
 /// Load the AST from a source-file, which is supposed to be located inside the

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 809440b196fd8..eb15c17870e33 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3765,12 +3765,6 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
   }
 
   if (HaveModules) {
-    // -fprebuilt-module-path specifies where to load the prebuilt module files.
-    for (const Arg *A : Args.filtered(options::OPT_fprebuilt_module_path)) {
-      CmdArgs.push_back(Args.MakeArgString(
-          std::string("-fprebuilt-module-path=") + A->getValue()));
-      A->claim();
-    }
     if (Args.hasFlag(options::OPT_fprebuilt_implicit_modules,
                      options::OPT_fno_prebuilt_implicit_modules, false))
       CmdArgs.push_back("-fprebuilt-implicit-modules");
@@ -3803,9 +3797,16 @@ static bool RenderModulesOptions(Compilation &C, const Driver &D,
   // names to precompiled module files (the module is loaded only if used).
   // The -fmodule-file=<file> form can be used to unconditionally load
   // precompiled module files (whether used or not).
-  if (HaveModules)
+  if (HaveModules || Input.getType() == clang::driver::types::TY_ModuleFile) {
     Args.AddAllArgs(CmdArgs, options::OPT_fmodule_file);
-  else
+
+    // -fprebuilt-module-path specifies where to load the prebuilt module files.
+    for (const Arg *A : Args.filtered(options::OPT_fprebuilt_module_path)) {
+      CmdArgs.push_back(Args.MakeArgString(
+          std::string("-fprebuilt-module-path=") + A->getValue()));
+      A->claim();
+    }
+  } else
     Args.ClaimAllArgs(options::OPT_fmodule_file);
 
   // When building modules and generating crashdumps, we need to dump a module

diff  --git a/clang/lib/Frontend/ASTMerge.cpp b/clang/lib/Frontend/ASTMerge.cpp
index 14d781ccdf936..057ea4fd5bb35 100644
--- a/clang/lib/Frontend/ASTMerge.cpp
+++ b/clang/lib/Frontend/ASTMerge.cpp
@@ -48,7 +48,7 @@ void ASTMergeAction::ExecuteAction() {
                                     /*ShouldOwnClient=*/true));
     std::unique_ptr<ASTUnit> Unit = ASTUnit::LoadFromASTFile(
         ASTFiles[I], CI.getPCHContainerReader(), ASTUnit::LoadEverything, Diags,
-        CI.getFileSystemOpts(), false);
+        CI.getFileSystemOpts(), CI.getHeaderSearchOptsPtr(), false);
 
     if (!Unit)
       continue;

diff  --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index d23a40f6ffd14..32ae76d673b6c 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -785,7 +785,8 @@ void ASTUnit::ConfigureDiags(IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
 std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
     const std::string &Filename, const PCHContainerReader &PCHContainerRdr,
     WhatToLoad ToLoad, IntrusiveRefCntPtr<DiagnosticsEngine> Diags,
-    const FileSystemOptions &FileSystemOpts, bool UseDebugInfo,
+    const FileSystemOptions &FileSystemOpts,
+    std::shared_ptr<HeaderSearchOptions> HSOpts, bool UseDebugInfo,
     bool OnlyLocalDecls, CaptureDiagsKind CaptureDiagnostics,
     bool AllowASTWithCompilerErrors, bool UserFilesAreVolatile,
     IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS) {
@@ -810,7 +811,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
                                      AST->getFileManager(),
                                      UserFilesAreVolatile);
   AST->ModuleCache = new InMemoryModuleCache;
-  AST->HSOpts = std::make_shared<HeaderSearchOptions>();
+  AST->HSOpts = HSOpts ? HSOpts : std::make_shared<HeaderSearchOptions>();
   AST->HSOpts->ModuleFormat = std::string(PCHContainerRdr.getFormats().front());
   AST->HeaderInfo.reset(new HeaderSearch(AST->HSOpts,
                                          AST->getSourceManager(),

diff  --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index c5f3efc8c93d8..a785ad88f122f 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -612,7 +612,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
     std::unique_ptr<ASTUnit> AST = ASTUnit::LoadFromASTFile(
         std::string(InputFile), CI.getPCHContainerReader(),
         ASTUnit::LoadPreprocessorOnly, ASTDiags, CI.getFileSystemOpts(),
-        CI.getCodeGenOpts().DebugTypeExtRefs);
+        /*HeaderSearchOptions=*/nullptr, CI.getCodeGenOpts().DebugTypeExtRefs);
     if (!AST)
       return false;
 
@@ -680,6 +680,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
     std::unique_ptr<ASTUnit> AST = ASTUnit::LoadFromASTFile(
         std::string(InputFile), CI.getPCHContainerReader(),
         ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts(),
+        CI.getHeaderSearchOptsPtr(),
         CI.getCodeGenOpts().DebugTypeExtRefs);
 
     if (!AST)

diff  --git a/clang/test/Modules/no-implicit-std-cxx-module.cppm b/clang/test/Modules/no-implicit-std-cxx-module.cppm
index 3e2b9722598a8..d7a9e3ce67c08 100644
--- a/clang/test/Modules/no-implicit-std-cxx-module.cppm
+++ b/clang/test/Modules/no-implicit-std-cxx-module.cppm
@@ -10,6 +10,17 @@
 // RUN:     -Wno-read-modules-implicitly -DNO_DIAG 
 // RUN: %clang_cc1 -std=c++20 %t/user.cpp -fmodule-file=a=%t/a.pcm -fmodule-file=b=%t/b.pcm \
 // RUN:     -DNO_DIAG -verify -fsyntax-only
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.pcm -S -emit-llvm -o - 2>&1 | FileCheck %t/a.cppm
+// RUN: %clang_cc1 -std=c++20 %t/a.pcm -fmodule-file=b=%t/b.pcm -S -emit-llvm -o - 2>&1 \
+// RUN:     | FileCheck %t/a.cppm -check-prefix=CHECK-CORRECT
+//
+// RUN: mkdir -p %t/tmp
+// RUN: mv %t/b.pcm %t/tmp/b.pcm
+// RUN: not %clang_cc1 -std=c++20 %t/a.pcm -S -emit-llvm -o - 2>&1 \
+// RUN:     | FileCheck %t/a.cppm -check-prefix=CHECK-ERROR
+// RUN: %clang_cc1 -std=c++20 %t/a.pcm -S -emit-llvm -o - 2>&1 -fmodule-file=b=%t/tmp/b.pcm \
+// RUN:     | FileCheck %t/a.cppm -check-prefix=CHECK-CORRECT
 
 //--- b.cppm
 export module b;
@@ -24,6 +35,14 @@ export int a() {
     return b() + 43;
 }
 
+// CHECK: it is deprecated to read module 'b' implcitly;
+
+// CHECK-CORRECT-NOT: warning
+// CHECK-CORRECT-NOT: error
+
+// CHECK-ERROR: error: module file{{.*}}not found: module file not found
+
+
 //--- user.cpp
 #ifdef NO_DIAG
 // expected-no-diagnostics

diff  --git a/clang/tools/c-index-test/core_main.cpp b/clang/tools/c-index-test/core_main.cpp
index ea15b2bcd5c22..6e9bbd044fd93 100644
--- a/clang/tools/c-index-test/core_main.cpp
+++ b/clang/tools/c-index-test/core_main.cpp
@@ -270,11 +270,13 @@ static bool printSourceSymbolsFromModule(StringRef modulePath,
     return true;
   }
 
+  auto HSOpts = std::make_shared<HeaderSearchOptions>();
+
   IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
       CompilerInstance::createDiagnostics(new DiagnosticOptions());
   std::unique_ptr<ASTUnit> AU = ASTUnit::LoadFromASTFile(
       std::string(modulePath), *pchRdr, ASTUnit::LoadASTOnly, Diags,
-      FileSystemOpts, /*UseDebugInfo=*/false,
+      FileSystemOpts, HSOpts, /*UseDebugInfo=*/false,
       /*OnlyLocalDecls=*/true, CaptureDiagsKind::None,
       /*AllowASTWithCompilerErrors=*/true,
       /*UserFilesAreVolatile=*/false);

diff  --git a/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp b/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
index 2a1e605ec4fc3..9ad16dc09ce18 100644
--- a/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ b/clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -153,7 +153,8 @@ static bool HandleAST(StringRef AstPath) {
 
   std::unique_ptr<ASTUnit> Unit = ASTUnit::LoadFromASTFile(
       AstPath.str(), CI->getPCHContainerOperations()->getRawReader(),
-      ASTUnit::LoadASTOnly, DiagEngine, CI->getFileSystemOpts());
+      ASTUnit::LoadASTOnly, DiagEngine, CI->getFileSystemOpts(),
+      CI->getHeaderSearchOptsPtr());
 
   if (!Unit)
     return false;

diff  --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp
index 448681bfe533a..ed2ca6ae2cfe1 100644
--- a/clang/tools/libclang/CIndex.cpp
+++ b/clang/tools/libclang/CIndex.cpp
@@ -3796,14 +3796,15 @@ enum CXErrorCode clang_createTranslationUnit2(CXIndex CIdx,
 
   CIndexer *CXXIdx = static_cast<CIndexer *>(CIdx);
   FileSystemOptions FileSystemOpts;
+  auto HSOpts = std::make_shared<HeaderSearchOptions>();
 
   IntrusiveRefCntPtr<DiagnosticsEngine> Diags =
       CompilerInstance::createDiagnostics(new DiagnosticOptions());
   std::unique_ptr<ASTUnit> AU = ASTUnit::LoadFromASTFile(
       ast_filename, CXXIdx->getPCHContainerOperations()->getRawReader(),
-      ASTUnit::LoadEverything, Diags, FileSystemOpts, /*UseDebugInfo=*/false,
-      CXXIdx->getOnlyLocalDecls(), CaptureDiagsKind::All,
-      /*AllowASTWithCompilerErrors=*/true,
+      ASTUnit::LoadEverything, Diags, FileSystemOpts, HSOpts,
+      /*UseDebugInfo=*/false, CXXIdx->getOnlyLocalDecls(),
+      CaptureDiagsKind::All, /*AllowASTWithCompilerErrors=*/true,
       /*UserFilesAreVolatile=*/true);
   *out_TU = MakeCXTranslationUnit(CXXIdx, std::move(AU));
   return *out_TU ? CXError_Success : CXError_Failure;

diff  --git a/clang/unittests/Frontend/ASTUnitTest.cpp b/clang/unittests/Frontend/ASTUnitTest.cpp
index d9d4932b8fee0..bb3466575d8a7 100644
--- a/clang/unittests/Frontend/ASTUnitTest.cpp
+++ b/clang/unittests/Frontend/ASTUnitTest.cpp
@@ -89,10 +89,11 @@ TEST_F(ASTUnitTest, SaveLoadPreservesLangOptionsInPrintingPolicy) {
   AST->Save(ASTFileName.str());
 
   EXPECT_TRUE(llvm::sys::fs::exists(ASTFileName));
+  auto HSOpts = std::make_shared<HeaderSearchOptions>();
 
   std::unique_ptr<ASTUnit> AU = ASTUnit::LoadFromASTFile(
       std::string(ASTFileName.str()), PCHContainerOps->getRawReader(),
-      ASTUnit::LoadEverything, Diags, FileSystemOptions(),
+      ASTUnit::LoadEverything, Diags, FileSystemOptions(), HSOpts,
       /*UseDebugInfo=*/false);
 
   if (!AU)


        


More information about the cfe-commits mailing list