Skip to content

Commit

Permalink
[C++20] [Modules] Don't ignore -fmodule-file when we compile pcm files
Browse files Browse the repository at this point in the history
Close #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
#62707.
  • Loading branch information
ChuanqiXu9 committed May 23, 2023
1 parent 53064c5 commit 807aa26
Show file tree
Hide file tree
Showing 11 changed files with 47 additions and 19 deletions.
1 change: 1 addition & 0 deletions clang/include/clang/Frontend/ASTUnit.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/CrossTU/CrossTranslationUnit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 9 additions & 8 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Frontend/ASTMerge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Frontend/ASTUnit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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(),
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Frontend/FrontendAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
Expand Down
19 changes: 19 additions & 0 deletions clang/test/Modules/no-implicit-std-cxx-module.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion clang/tools/c-index-test/core_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions clang/tools/libclang/CIndex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion clang/unittests/Frontend/ASTUnitTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 807aa26

Please sign in to comment.