Skip to content

Commit b2b463b

Browse files
committed
[C++20] [Modules] Add signature to the BMI recording export imported
modules After #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.
1 parent 7ac1fb0 commit b2b463b

File tree

4 files changed

+114
-26
lines changed

4 files changed

+114
-26
lines changed

clang/include/clang/Serialization/ASTWriter.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,7 @@ class ASTWriter : public ASTDeserializationListener,
525525

526526
/// Calculate hash of the pcm content.
527527
std::pair<ASTFileSignature, ASTFileSignature> createSignature() const;
528+
ASTFileSignature createSignatureForNamedModule() const;
528529

529530
void WriteInputFiles(SourceManager &SourceMgr, HeaderSearchOptions &HSOpts);
530531
void WriteSourceManagerBlock(SourceManager &SourceMgr,

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 49 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1174,26 +1174,47 @@ ASTWriter::createSignature() const {
11741174
return std::make_pair(ASTBlockHash, Signature);
11751175
}
11761176

1177+
ASTFileSignature ASTWriter::createSignatureForNamedModule() const {
1178+
llvm::SHA1 Hasher;
1179+
Hasher.update(StringRef(Buffer.data(), Buffer.size()));
1180+
1181+
assert(WritingModule);
1182+
assert(WritingModule->isNamedModule());
1183+
1184+
// We need to combine all the export imported modules no matter
1185+
// we used it or not.
1186+
for (auto [ExportImported, _] : WritingModule->Exports)
1187+
Hasher.update(ExportImported->Signature);
1188+
1189+
return ASTFileSignature::create(Hasher.result());
1190+
}
1191+
1192+
static void BackpatchSignatureAt(llvm::BitstreamWriter &Stream,
1193+
const ASTFileSignature &S, uint64_t BitNo) {
1194+
for (uint8_t Byte : S) {
1195+
Stream.BackpatchByte(BitNo, Byte);
1196+
BitNo += 8;
1197+
}
1198+
}
1199+
11771200
ASTFileSignature ASTWriter::backpatchSignature() {
1201+
if (isWritingStdCXXNamedModules()) {
1202+
ASTFileSignature Signature = createSignatureForNamedModule();
1203+
BackpatchSignatureAt(Stream, Signature, SignatureOffset);
1204+
return Signature;
1205+
}
1206+
11781207
if (!WritingModule ||
11791208
!PP->getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent)
11801209
return {};
11811210

11821211
// For implicit modules, write the hash of the PCM as its signature.
1183-
1184-
auto BackpatchSignatureAt = [&](const ASTFileSignature &S, uint64_t BitNo) {
1185-
for (uint8_t Byte : S) {
1186-
Stream.BackpatchByte(BitNo, Byte);
1187-
BitNo += 8;
1188-
}
1189-
};
1190-
11911212
ASTFileSignature ASTBlockHash;
11921213
ASTFileSignature Signature;
11931214
std::tie(ASTBlockHash, Signature) = createSignature();
11941215

1195-
BackpatchSignatureAt(ASTBlockHash, ASTBlockHashOffset);
1196-
BackpatchSignatureAt(Signature, SignatureOffset);
1216+
BackpatchSignatureAt(Stream, ASTBlockHash, ASTBlockHashOffset);
1217+
BackpatchSignatureAt(Stream, Signature, SignatureOffset);
11971218

11981219
return Signature;
11991220
}
@@ -1210,9 +1231,11 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
12101231
RecordData Record;
12111232
Stream.EnterSubblock(UNHASHED_CONTROL_BLOCK_ID, 5);
12121233

1213-
// For implicit modules, write the hash of the PCM as its signature.
1214-
if (WritingModule &&
1215-
PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent) {
1234+
// For implicit modules and C++20 named modules, write the hash of the PCM as
1235+
// its signature.
1236+
if (isWritingStdCXXNamedModules() ||
1237+
(WritingModule &&
1238+
PP.getHeaderSearchInfo().getHeaderSearchOpts().ModulesHashContent)) {
12161239
// At this point, we don't know the actual signature of the file or the AST
12171240
// block - we're only able to compute those at the end of the serialization
12181241
// process. Let's store dummy signatures for now, and replace them with the
@@ -1223,21 +1246,24 @@ void ASTWriter::writeUnhashedControlBlock(Preprocessor &PP,
12231246
auto Dummy = ASTFileSignature::createDummy();
12241247
SmallString<128> Blob{Dummy.begin(), Dummy.end()};
12251248

1226-
auto Abbrev = std::make_shared<BitCodeAbbrev>();
1227-
Abbrev->Add(BitCodeAbbrevOp(AST_BLOCK_HASH));
1228-
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
1229-
unsigned ASTBlockHashAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
1249+
// We don't need AST Block hash in named modules.
1250+
if (!isWritingStdCXXNamedModules()) {
1251+
auto Abbrev = std::make_shared<BitCodeAbbrev>();
1252+
Abbrev->Add(BitCodeAbbrevOp(AST_BLOCK_HASH));
1253+
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
1254+
unsigned ASTBlockHashAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
12301255

1231-
Abbrev = std::make_shared<BitCodeAbbrev>();
1256+
Record.push_back(AST_BLOCK_HASH);
1257+
Stream.EmitRecordWithBlob(ASTBlockHashAbbrev, Record, Blob);
1258+
ASTBlockHashOffset = Stream.GetCurrentBitNo() - Blob.size() * 8;
1259+
Record.clear();
1260+
}
1261+
1262+
auto Abbrev = std::make_shared<BitCodeAbbrev>();
12321263
Abbrev->Add(BitCodeAbbrevOp(SIGNATURE));
12331264
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
12341265
unsigned SignatureAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
12351266

1236-
Record.push_back(AST_BLOCK_HASH);
1237-
Stream.EmitRecordWithBlob(ASTBlockHashAbbrev, Record, Blob);
1238-
ASTBlockHashOffset = Stream.GetCurrentBitNo() - Blob.size() * 8;
1239-
Record.clear();
1240-
12411267
Record.push_back(SIGNATURE);
12421268
Stream.EmitRecordWithBlob(SignatureAbbrev, Record, Blob);
12431269
SignatureOffset = Stream.GetCurrentBitNo() - Blob.size() * 8;
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Test that the changes from export imported modules and touched
2+
// modules can be popullated as expected.
3+
//
4+
// RUN: rm -rf %t
5+
// RUN: split-file %s %t
6+
// RUN: cd %t
7+
//
8+
// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm
9+
// RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-reduced-module-interface -o %t/A.v1.pcm
10+
11+
// The BMI of B should change it export imports A, so all the change to A should be popullated
12+
// to B.
13+
// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.pcm \
14+
// RUN: -o %t/B.pcm
15+
// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.v1.pcm \
16+
// RUN: -o %t/B.v1.pcm
17+
// RUN: not diff %t/B.v1.pcm %t/B.pcm &> /dev/null
18+
19+
//--- A.cppm
20+
export module A;
21+
export int funcA() {
22+
return 43;
23+
}
24+
25+
//--- A.v1.cppm
26+
export module A;
27+
28+
export int funcA() {
29+
return 43;
30+
}
31+
32+
//--- B.cppm
33+
export module B;
34+
export import A;
35+
36+
export int funcB() {
37+
return funcA();
38+
}

clang/test/Modules/no-transitive-source-location-change.cppm

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66
// RUN: cd %t
77
//
88
// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm
9-
10-
//
119
// RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-module-interface -o %t/A.v1.pcm
1210
//
1311
// The BMI may not be the same since the source location differs.
@@ -26,6 +24,31 @@
2624
// RUN: %clang_cc1 -std=c++20 %t/C.cppm -emit-module-interface -fmodule-file=A=%t/A.v1.pcm \
2725
// RUN: -o %t/C.v1.pcm
2826
// RUN: not diff %t/C.v1.pcm %t/C.pcm &> /dev/null
27+
//
28+
// RUN: rm -rf %t
29+
// RUN: split-file %s %t
30+
// RUN: cd %t
31+
//
32+
// Test again with reduced BMI.
33+
// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm
34+
// RUN: %clang_cc1 -std=c++20 %t/A.v1.cppm -emit-reduced-module-interface -o %t/A.v1.pcm
35+
//
36+
// The BMI may not be the same since the source location differs.
37+
// RUN: not diff %t/A.pcm %t/A.v1.pcm &> /dev/null
38+
//
39+
// The BMI of B shouldn't change since all the locations remain the same.
40+
// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.pcm \
41+
// RUN: -o %t/B.pcm
42+
// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.v1.pcm \
43+
// RUN: -o %t/B.v1.pcm
44+
// RUN: diff %t/B.v1.pcm %t/B.pcm &> /dev/null
45+
//
46+
// The BMI of C may change since the locations for instantiations changes.
47+
// RUN: %clang_cc1 -std=c++20 %t/C.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.pcm \
48+
// RUN: -o %t/C.pcm
49+
// RUN: %clang_cc1 -std=c++20 %t/C.cppm -emit-reduced-module-interface -fmodule-file=A=%t/A.v1.pcm \
50+
// RUN: -o %t/C.v1.pcm
51+
// RUN: not diff %t/C.v1.pcm %t/C.pcm &> /dev/null
2952

3053
//--- A.cppm
3154
export module A;
@@ -63,7 +86,7 @@ export int funcB() {
6386
//--- C.cppm
6487
export module C;
6588
import A;
66-
export void testD() {
89+
export inline void testD() {
6790
C<int> c;
6891
c.func();
6992
}

0 commit comments

Comments
 (0)