https://github.com/qiongsiwu updated https://github.com/llvm/llvm-project/pull/180065
>From b8ba91c9cf72a34cb88d39de744fefe8711eeeac Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <[email protected]> Date: Thu, 5 Feb 2026 15:08:06 -0800 Subject: [PATCH 1/5] Canonicalizing -include-pch input in the frontend. --- clang/lib/Frontend/FrontendAction.cpp | 17 +++++++++--- clang/test/PCH/debug-info-pch-path.c | 2 ++ clang/test/PCH/pch-input-path-independent.c | 29 +++++++++++++++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 clang/test/PCH/pch-input-path-independent.c diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 7810f0999f7d6..c5c17334da295 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -1018,13 +1018,22 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, return true; } - // If the implicit PCH include is actually a directory, rather than - // a single file, search for a suitable PCH file in that directory. if (!CI.getPreprocessorOpts().ImplicitPCHInclude.empty()) { FileManager &FileMgr = CI.getFileManager(); PreprocessorOptions &PPOpts = CI.getPreprocessorOpts(); + + // Canonicalize ImplicitPCHInclude. This way, all the downstream code, + // including the ASTWriter, will receive the absolute path to the included + // PCH. This way we can avoid reasoning about absolute path or relative + // paths later on during serialization. + SmallString<128> PCHIncludePath(PPOpts.ImplicitPCHInclude); + FileMgr.makeAbsolutePath(PCHIncludePath); + llvm::sys::path::remove_dots(PCHIncludePath, true); + PPOpts.ImplicitPCHInclude = PCHIncludePath.str(); StringRef PCHInclude = PPOpts.ImplicitPCHInclude; - std::string SpecificModuleCachePath = CI.getSpecificModuleCachePath(); + + // If the implicit PCH include is actually a directory, rather than + // a single file, search for a suitable PCH file in that directory. if (auto PCHDir = FileMgr.getOptionalDirectoryRef(PCHInclude)) { std::error_code EC; SmallString<128> DirNative; @@ -1040,7 +1049,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, CI.getPCHContainerReader(), CI.getLangOpts(), CI.getCodeGenOpts(), CI.getTargetOpts(), CI.getPreprocessorOpts(), CI.getHeaderSearchOpts(), - SpecificModuleCachePath, + CI.getSpecificModuleCachePath(), /*RequireStrictOptionMatches=*/true)) { PPOpts.ImplicitPCHInclude = std::string(Dir->path()); Found = true; diff --git a/clang/test/PCH/debug-info-pch-path.c b/clang/test/PCH/debug-info-pch-path.c index 22b367f344204..0a4e9d8146f00 100644 --- a/clang/test/PCH/debug-info-pch-path.c +++ b/clang/test/PCH/debug-info-pch-path.c @@ -17,6 +17,7 @@ // RUN: %clang_cc1 -debug-info-kind=standalone \ // RUN: -dwarf-ext-refs -fmodule-format=obj \ // RUN: -triple %itanium_abi_triple \ +// RUN: -fdebug-compilation-dir=%t \ // RUN: -include-pch prefix.pch %s -emit-llvm -o %t.nodir.ll %s // RUN: cat %t.nodir.ll | FileCheck %s --check-prefix=CHECK-REL-NODIR // @@ -42,6 +43,7 @@ // RUN: %clang_cc1 -debug-info-kind=standalone \ // RUN: -dwarf-ext-refs -fmodule-format=obj \ // RUN: -triple %itanium_abi_triple \ +// RUN: -fdebug-compilation-dir=%t \ // RUN: -include-pch pchdir/prefix.pch %s -emit-llvm -o %t.rel.ll %s // RUN: cat %t.rel.ll | FileCheck %s --check-prefix=CHECK-REL diff --git a/clang/test/PCH/pch-input-path-independent.c b/clang/test/PCH/pch-input-path-independent.c new file mode 100644 index 0000000000000..56a697b9d300f --- /dev/null +++ b/clang/test/PCH/pch-input-path-independent.c @@ -0,0 +1,29 @@ +// Testing `-include-pch` canoniclaization. +// When PCHs are chained, the dependent PCHs produced are identical +// whether the included PCH is specified through a relative path or an absolute +// path (bridging1.h.pch vs bridging2.h.pch). +// The dependent PCHs are also identical regardless of the working +// directory where clang is invoked (bridging1.h.pch vs bridging3.h.pch). + +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: cd %t +// RUN: %clang_cc1 -triple x86_64-apple-macos11 -emit-pch h1.h -o %t/h1.h.pch +// RUN: %clang_cc1 -triple x86_64-apple-macos11 -emit-pch %t/bridging.h \ +// RUN: -o %t/bridging1.h.pch -include-pch %t/h1.h.pch +// RUN: %clang_cc1 -triple x86_64-apple-macos11 -emit-pch %t/bridging.h \ +// RUN: -o %t/bridging2.h.pch -include-pch ./h1.h.pch +// RUN: mkdir %t/wd/ +// RUN: cd %t/wd/ +// RUN: %clang_cc1 -triple x86_64-apple-macos11 -emit-pch %t/bridging.h \ +// RUN: -o %t/bridging3.h.pch -include-pch ../h1.h.pch + +// RUN: diff %t/bridging1.h.pch %t/bridging2.h.pch +// RUN: diff %t/bridging1.h.pch %t/bridging3.h.pch + +//--- h1.h +int bar1() { return 42; } + +//--- bridging.h +int bar() { return bar1(); } + >From 60271a3d0fbf22e28757da727828e6c2de2d1d74 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <[email protected]> Date: Thu, 5 Feb 2026 15:51:35 -0800 Subject: [PATCH 2/5] Fixing tests on Windows. --- clang/test/Modules/validate-file-content.m | 2 +- clang/test/PCH/modified-module-dependency.m | 2 +- clang/test/PCH/validate-file-content.m | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/test/Modules/validate-file-content.m b/clang/test/Modules/validate-file-content.m index cff89884552b7..1eae7748165c1 100644 --- a/clang/test/Modules/validate-file-content.m +++ b/clang/test/Modules/validate-file-content.m @@ -24,7 +24,7 @@ // RUN: not %clang_cc1 -fsyntax-only -fmodules-cache-path=%t/cache -fmodules -fimplicit-module-maps -I %t -include-pch %t/a.pch %s -fvalidate-ast-input-files-content 2> %t/stderr // RUN: FileCheck %s < %t/stderr // -// CHECK: file '[[M_H:.*[/\\]m\.h]]' has been modified since the precompiled header '[[A_PCH:.*/a\.pch]]' was built: content changed +// CHECK: file '[[M_H:.*[/\\]m\.h]]' has been modified since the precompiled header '[[A_PCH:.*[/\\]a\.pch]]' was built: content changed // CHECK: '[[M_H]]' required by '[[M_PCM:.*[/\\]m.*\.pcm]]' // CHECK: '[[M_PCM]]' required by '[[A_PCH]]' // CHECK: please rebuild precompiled file '[[A_PCH]]' diff --git a/clang/test/PCH/modified-module-dependency.m b/clang/test/PCH/modified-module-dependency.m index a4710dea51169..bf93b53e2c152 100644 --- a/clang/test/PCH/modified-module-dependency.m +++ b/clang/test/PCH/modified-module-dependency.m @@ -14,7 +14,7 @@ // RUN: not %clang_cc1 -x objective-c -I %t-dir -include-pch %t-dir/prefix.pch -fmodules -fimplicit-module-maps -fmodules-cache-path=%t-dir/cache -fdisable-module-hash -fsyntax-only %s 2> %t-dir/log // RUN: FileCheck %s < %t-dir/log -// CHECK: file '[[TEST_H:.*[/\\]test\.h]]' has been modified since the precompiled header '[[PREFIX_PCH:.*/prefix\.pch]]' was built +// CHECK: file '[[TEST_H:.*[/\\]test\.h]]' has been modified since the precompiled header '[[PREFIX_PCH:.*[/\\]prefix\.pch]]' was built // CHECK: '[[TEST_H]]' required by '[[TEST_PCM:.*[/\\]test\.pcm]]' // CHECK: '[[TEST_PCM]]' required by '[[PREFIX_PCH]]' // CHECK: please rebuild precompiled file '[[PREFIX_PCH]]' diff --git a/clang/test/PCH/validate-file-content.m b/clang/test/PCH/validate-file-content.m index 8863b7abea3af..289dbff1a8c20 100644 --- a/clang/test/PCH/validate-file-content.m +++ b/clang/test/PCH/validate-file-content.m @@ -22,6 +22,6 @@ // RUN: not %clang_cc1 -fsyntax-only -I %t -include-pch %t/a.pch %s -fvalidate-ast-input-files-content 2> %t/stderr // RUN: FileCheck %s < %t/stderr // -// CHECK: file '[[M_H:.*[/\\]m\.h]]' has been modified since the precompiled header '[[A_PCH:.*/a\.pch]]' was built: content changed +// CHECK: file '[[M_H:.*[/\\]m\.h]]' has been modified since the precompiled header '[[A_PCH:.*[/\\]a\.pch]]' was built: content changed // CHECK: please rebuild precompiled file '[[A_PCH]]' // expected-no-diagnostics >From 1537d317ef5de720ae2f5170f5e555f75802f2da Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <[email protected]> Date: Thu, 5 Feb 2026 16:06:41 -0800 Subject: [PATCH 3/5] Address code review comments. --- clang/lib/Frontend/FrontendAction.cpp | 3 ++- clang/test/PCH/pch-input-path-independent.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index c5c17334da295..94b03016198b4 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -1040,6 +1040,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, llvm::sys::path::native(PCHDir->getName(), DirNative); bool Found = false; llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem(); + std::string SpecificModuleCachePath = CI.getSpecificModuleCachePath(); for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd; Dir != DirEnd && !EC; Dir.increment(EC)) { @@ -1049,7 +1050,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, CI.getPCHContainerReader(), CI.getLangOpts(), CI.getCodeGenOpts(), CI.getTargetOpts(), CI.getPreprocessorOpts(), CI.getHeaderSearchOpts(), - CI.getSpecificModuleCachePath(), + SpecificModuleCachePath, /*RequireStrictOptionMatches=*/true)) { PPOpts.ImplicitPCHInclude = std::string(Dir->path()); Found = true; diff --git a/clang/test/PCH/pch-input-path-independent.c b/clang/test/PCH/pch-input-path-independent.c index 56a697b9d300f..44d01e4c4040c 100644 --- a/clang/test/PCH/pch-input-path-independent.c +++ b/clang/test/PCH/pch-input-path-independent.c @@ -1,4 +1,4 @@ -// Testing `-include-pch` canoniclaization. +// Testing `-include-pch` canonicalization. // When PCHs are chained, the dependent PCHs produced are identical // whether the included PCH is specified through a relative path or an absolute // path (bridging1.h.pch vs bridging2.h.pch). >From 767f8d16d559fbcc33ba29341b924fd060a1df78 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <[email protected]> Date: Fri, 6 Feb 2026 10:22:59 -0800 Subject: [PATCH 4/5] Address code review comments. --- clang/lib/Frontend/FrontendAction.cpp | 8 ++++++-- clang/lib/Serialization/ASTWriter.cpp | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 94b03016198b4..92a7220a0d2e3 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -1024,8 +1024,12 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, // Canonicalize ImplicitPCHInclude. This way, all the downstream code, // including the ASTWriter, will receive the absolute path to the included - // PCH. This way we can avoid reasoning about absolute path or relative - // paths later on during serialization. + // PCH. + // FIXME: When serializing ImplicitPCHInclude, the ASTWriter + // currently does not handle the relocatable AST case. The ASTWriter + // simply seralizes the PCH path as it is. This implies that including + // a PCH in another PCH may not work with -relocatable-pch. Revisit + // if a use case of chaining relocatable PCHs arises. SmallString<128> PCHIncludePath(PPOpts.ImplicitPCHInclude); FileMgr.makeAbsolutePath(PCHIncludePath); llvm::sys::path::remove_dots(PCHIncludePath, true); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 3e10bbfedfe65..71e31ff0fe271 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1752,6 +1752,11 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { Record.push_back(PPOpts.UsePredefines); // Detailed record is important since it is used for the module cache hash. Record.push_back(PPOpts.DetailedRecord); + + // FIXME: When serializing ImplicitPCHInclude, the ASTWriter + // currently does not handle the relocatable AST case. We probably should + // call AddPath(PPOpts.ImplicitPCHInclude, Record) to properly support chained + // relocatable PCHs. AddString(PPOpts.ImplicitPCHInclude, Record); Record.push_back(static_cast<unsigned>(PPOpts.ObjCXXARCStandardLibrary)); Stream.EmitRecord(PREPROCESSOR_OPTIONS, Record); @@ -6115,6 +6120,9 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema *SemaPtr, StringRef isysroot, endian::Writer LE(Out, llvm::endianness::little); LE.write<uint8_t>(static_cast<uint8_t>(M.Kind)); + // FIXME: The ASTWriter currently does not handle chained relocatable + // PCHs. We probably should call PreparePathForOutput(M.FileName) to + // properly support chained relocatable PCHs. StringRef Name = M.isModule() ? M.ModuleName : M.FileName; LE.write<uint16_t>(Name.size()); Out.write(Name.data(), Name.size()); >From 7819530f21a0b9669b5cc4907c9c5b244168ea96 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu <[email protected]> Date: Fri, 6 Feb 2026 11:48:19 -0800 Subject: [PATCH 5/5] Address review comments. --- clang/lib/Frontend/FrontendAction.cpp | 5 ----- clang/lib/Serialization/ASTWriter.cpp | 12 ++++++------ 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 92a7220a0d2e3..79e862f01be14 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -1025,11 +1025,6 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, // Canonicalize ImplicitPCHInclude. This way, all the downstream code, // including the ASTWriter, will receive the absolute path to the included // PCH. - // FIXME: When serializing ImplicitPCHInclude, the ASTWriter - // currently does not handle the relocatable AST case. The ASTWriter - // simply seralizes the PCH path as it is. This implies that including - // a PCH in another PCH may not work with -relocatable-pch. Revisit - // if a use case of chaining relocatable PCHs arises. SmallString<128> PCHIncludePath(PPOpts.ImplicitPCHInclude); FileMgr.makeAbsolutePath(PCHIncludePath); llvm::sys::path::remove_dots(PCHIncludePath, true); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 71e31ff0fe271..07740a65821f3 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1753,9 +1753,9 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { // Detailed record is important since it is used for the module cache hash. Record.push_back(PPOpts.DetailedRecord); - // FIXME: When serializing ImplicitPCHInclude, the ASTWriter - // currently does not handle the relocatable AST case. We probably should - // call AddPath(PPOpts.ImplicitPCHInclude, Record) to properly support chained + // FIXME: Using `AddString` to record `ImplicitPCHInclude` does not handle + // relocatable files. We probably should call + // `AddPath(PPOpts.ImplicitPCHInclude, Record)` to properly support chained // relocatable PCHs. AddString(PPOpts.ImplicitPCHInclude, Record); Record.push_back(static_cast<unsigned>(PPOpts.ObjCXXARCStandardLibrary)); @@ -6120,9 +6120,9 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema *SemaPtr, StringRef isysroot, endian::Writer LE(Out, llvm::endianness::little); LE.write<uint8_t>(static_cast<uint8_t>(M.Kind)); - // FIXME: The ASTWriter currently does not handle chained relocatable - // PCHs. We probably should call PreparePathForOutput(M.FileName) to - // properly support chained relocatable PCHs. + // FIXME: Storing Name as just a string does not handle relocatable + // files. We probably should call `PreparePathForOutput(...)` to + // properly support relocatable files. StringRef Name = M.isModule() ? M.ModuleName : M.FileName; LE.write<uint16_t>(Name.size()); Out.write(Name.data(), Name.size()); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
