[clang] b2b463b - [C++20] [Modules] Add signature to the BMI recording export imported
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 30 01:35:17 PDT 2024
Author: Chuanqi Xu
Date: 2024-04-30T16:33:34+08:00
New Revision: b2b463bd8f6b21f040b80c4493682cf74f8dced5
URL: https://github.com/llvm/llvm-project/commit/b2b463bd8f6b21f040b80c4493682cf74f8dced5
DIFF: https://github.com/llvm/llvm-project/commit/b2b463bd8f6b21f040b80c4493682cf74f8dced5.diff
LOG: [C++20] [Modules] Add signature to the BMI recording export imported
modules
After https://github.com/llvm/llvm-project/pull/86912,
for the following example,
```
export module A;
export import B;
```
The generated BMI of `A` won't change if the source location in `A`
changes. Further, we plan avoid more such changes.
However, it is slightly problematic since `export import` should
propagate all the changes.
So this patch adds a signature to the BMI of C++20 modules so that we
can propagate the changes correctly.
Added:
clang/test/Modules/force-transitive-changes.cppm
Modified:
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/no-transitive-source-location-change.cppm
Removed:
################################################################################
diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index 428bf6a5a791b3..921678d278d6e2 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -525,6 +525,7 @@ class ASTWriter : public ASTDeserializationListener,
/// Calculate hash of the pcm content.
std::pair<ASTFileSignature, ASTFileSignature> createSignature() const;
+ ASTFileSignature createSignatureForNamedModule() const;
void WriteInputFiles(SourceManager &SourceMgr, HeaderSearchOptions &HSOpts);
void WriteSourceManagerBlock(SourceManager &SourceMgr,
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 4d85f6eb10d232..c3fcd1a4df2368 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -1174,26 +1174,47 @@ ASTWriter::createSignature() const {
return std::make_pair(ASTBlockHash, Signature);
}
+ASTFileSignature ASTWriter::createSignatureForNamedModule() const {
+ llvm::SHA1 Hasher;
+ Hasher.update(StringRef(Buffer.data(), Buffer.size()));
+
+ assert(WritingModule);
+ assert(WritingModule->isNamedModule());
+
+ // We need to combine all the export imported modules no matter
+ // we used it or not.
+ for (auto [ExportImported, _] : WritingModule->Exports)
+ Hasher.update(ExportImported->Signature);
+
+ return ASTFileSignature::create(Hasher.result());
+}
+
+static void BackpatchSignatureAt(llvm::BitstreamWriter &Stream,
+ const ASTFileSignature &S, uint64_t BitNo) {
+ for (uint8_t Byte : S) {
+ Stream.BackpatchByte(BitNo, Byte);
+ BitNo += 8;
+ }
+}
+
ASTFileSignature ASTWriter::backpatchSignature() {
+ if (isWritingStdCXXNamedModules()) {
+ ASTFileSignature Signature = createSignatureForNamedModule();
+ BackpatchSignatureAt(Stream, Signature, SignatureOffset);
+ return Signature;
+ }
+
if (!WritingModule ||
!PP->getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent)
return {};
// For implicit modules, write the hash of the PCM as its signature.
-
- auto BackpatchSignatureAt = [&](const ASTFileSignature &S, uint64_t BitNo) {
- for (uint8_t Byte : S) {
- Stream.BackpatchByte(BitNo, Byte);
- BitNo += 8;
- }
- };
-
ASTFileSignature ASTBlockHash;
ASTFileSignature Signature;
std::tie(ASTBlockHash, Signature) = createSignature();
- BackpatchSignatureAt(ASTBlockHash, ASTBlockHashOffset);
- BackpatchSignatureAt(Signature, SignatureOffset);
+ BackpatchSignatureAt(Stream, ASTBlockHash, ASTBlockHashOffset);
+ BackpatchSignatureAt(Stream, Signature, SignatureOffset);
return Signature;
}
@@ -1210,9 +1231,11 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
RecordData Record;
Stream.EnterSubblock(UNHASHED_CONTROL_BLOCK_ID, 5);
- // For implicit modules, write the hash of the PCM as its signature.
- if (WritingModule &&
- PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent) {
+ // For implicit modules and C++20 named modules, write the hash of the PCM as
+ // its signature.
+ if (isWritingStdCXXNamedModules() ||
+ (WritingModule &&
+ PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent)) {
// At this point, we don't know the actual signature of the file or the AST
// block - we're only able to compute those at the end of the serialization
// process. Let's store dummy signatures for now, and replace them with the
@@ -1223,21 +1246,24 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
auto Dummy = ASTFileSignature::createDummy();
SmallString<128> Blob{Dummy.begin(), Dummy.end()};
- auto Abbrev = std::make_shared<BitCodeAbbrev>();
- Abbrev->Add(BitCodeAbbrevOp(AST_BLOCK_HASH));
- Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
- unsigned ASTBlockHashAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
+ // We don't need AST Block hash in named modules.
+ if (!isWritingStdCXXNamedModules()) {
+ auto Abbrev = std::make_shared<BitCodeAbbrev>();
+ Abbrev->Add(BitCodeAbbrevOp(AST_BLOCK_HASH));
+ Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
+ unsigned ASTBlockHashAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
- Abbrev = std::make_shared<BitCodeAbbrev>();
+ Record.push_back(AST_BLOCK_HASH);
+ Stream.EmitRecordWithBlob(ASTBlockHashAbbrev, Record, Blob);
+ ASTBlockHashOffset = Stream.GetCurrentBitNo() - Blob.size() * 8;
+ Record.clear();
+ }
+
+ auto Abbrev = std::make_shared<BitCodeAbbrev>();
Abbrev->Add(BitCodeAbbrevOp(SIGNATURE));
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
unsigned SignatureAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
- Record.push_back(AST_BLOCK_HASH);
- Stream.EmitRecordWithBlob(ASTBlockHashAbbrev, Record, Blob);
- ASTBlockHashOffset = Stream.GetCurrentBitNo() - Blob.size() * 8;
- Record.clear();
-
Record.push_back(SIGNATURE);
Stream.EmitRecordWithBlob(SignatureAbbrev, Record, Blob);
SignatureOffset = Stream.GetCurrentBitNo() - Blob.size() * 8;
diff --git a/clang/test/Modules/force-transitive-changes.cppm b/clang/test/Modules/force-transitive-changes.cppm
new file mode 100644
index 00000000000000..5732d264d1d552
--- /dev/null
+++ b/clang/test/Modules/force-transitive-changes.cppm
@@ -0,0 +1,38 @@
+// Test that the changes from export imported modules and touched
+// modules can be popullated as expected.
+//
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-reduced-module-interface -o %t/A.v1.pcm
+
+// The BMI of B should change it export imports A, so all the change to A should be popullated
+// to B.
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.pcm \
+// RUN: -o %t/B.pcm
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.v1.pcm \
+// RUN: -o %t/B.v1.pcm
+// RUN: not
diff %t/B.v1.pcm %t/B.pcm &> /dev/null
+
+//--- A.cppm
+export module A;
+export int funcA() {
+ return 43;
+}
+
+//--- A.v1.cppm
+export module A;
+
+export int funcA() {
+ return 43;
+}
+
+//--- B.cppm
+export module B;
+export import A;
+
+export int funcB() {
+ return funcA();
+}
diff --git a/clang/test/Modules/no-transitive-source-location-change.cppm b/clang/test/Modules/no-transitive-source-location-change.cppm
index b876fbb6d9f0cf..83cf6fb4f684d0 100644
--- a/clang/test/Modules/no-transitive-source-location-change.cppm
+++ b/clang/test/Modules/no-transitive-source-location-change.cppm
@@ -6,8 +6,6 @@
// RUN: cd %t
//
// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm
-
-//
// RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-module-interface -o %t/A.v1.pcm
//
// The BMI may not be the same since the source location
diff ers.
@@ -26,6 +24,31 @@
// RUN: %clang_cc1 -std=c++20 %t/C.cppm -emit-module-interface -fmodule-file=A=%t/A.v1.pcm \
// RUN: -o %t/C.v1.pcm
// RUN: not
diff %t/C.v1.pcm %t/C.pcm &> /dev/null
+//
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+//
+// Test again with reduced BMI.
+// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm
+// RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-reduced-module-interface -o %t/A.v1.pcm
+//
+// The BMI may not be the same since the source location
diff ers.
+// RUN: not
diff %t/A.pcm %t/A.v1.pcm &> /dev/null
+//
+// The BMI of B shouldn't change since all the locations remain the same.
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.pcm \
+// RUN: -o %t/B.pcm
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.v1.pcm \
+// RUN: -o %t/B.v1.pcm
+// RUN:
diff %t/B.v1.pcm %t/B.pcm &> /dev/null
+//
+// The BMI of C may change since the locations for instantiations changes.
+// RUN: %clang_cc1 -std=c++20 %t/C.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.pcm \
+// RUN: -o %t/C.pcm
+// RUN: %clang_cc1 -std=c++20 %t/C.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.v1.pcm \
+// RUN: -o %t/C.v1.pcm
+// RUN: not
diff %t/C.v1.pcm %t/C.pcm &> /dev/null
//--- A.cppm
export module A;
@@ -63,7 +86,7 @@ export int funcB() {
//--- C.cppm
export module C;
import A;
-export void testD() {
+export inline void testD() {
C<int> c;
c.func();
}
More information about the cfe-commits
mailing list