iains updated this revision to Diff 413913.
iains added a comment.
updated to remove a single-use variable.
Having some difficulty in reproducing the CI fails outside the harness, however
the
code setting the removed variable could have faulted if there was no macro data
for
the identifier. Set the IsHeaderUnit variable explicitly.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121097/new/
https://reviews.llvm.org/D121097
Files:
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/cxx20-hu-04.cpp
Index: clang/test/Modules/cxx20-hu-04.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/cxx20-hu-04.cpp
@@ -0,0 +1,105 @@
+// Test macro preservation in C++20 Header Units.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header hu-01.h \
+// RUN: -o hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info hu-01.pcm | \
+// RUN: FileCheck --check-prefix=CHECK-HU %s -DTDIR=%t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header hu-02.h \
+// RUN: -DFOO -fmodule-file=hu-01.pcm -o hu-02.pcm -Rmodule-import 2>&1 | \
+// RUN: FileCheck --check-prefix=CHECK-IMP %s -DTDIR=%t
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info hu-02.pcm | \
+// RUN: FileCheck --check-prefix=CHECK-HU2 %s -DTDIR=%t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface importer-01.cpp \
+// RUN: -fmodule-file=hu-02.pcm -o B.pcm -DTDIR=%t -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface importer-02.cpp \
+// RUN: -fmodule-file=hu-02.pcm -o C.pcm -DTDIR=%t -Rmodule-import 2>&1 | \
+// RUN: FileCheck --check-prefix=CHECK-IMP-HU2 %s -DTDIR=%t
+
+//--- hu-01.h
+#ifndef __GUARD
+#define __GUARD
+
+int baz(int);
+#define FORTYTWO 42
+
+#define SHOULD_NOT_BE_DEFINED -1
+#undef SHOULD_NOT_BE_DEFINED
+
+#endif // __GUARD
+// expected-no-diagnostics
+
+// CHECK-HU: ====== C++20 Module structure ======
+// CHECK-HU-NEXT: Header Unit './hu-01.h' is the Primary Module at index #1
+
+//--- hu-02.h
+export import "hu-01.h";
+#if !defined(FORTYTWO) || FORTYTWO != 42
+#error FORTYTWO missing in hu-02
+#endif
+
+#ifndef __GUARD
+#error __GUARD missing in hu-02
+#endif
+
+#ifdef SHOULD_NOT_BE_DEFINED
+#error SHOULD_NOT_BE_DEFINED is visible
+#endif
+
+#define KAP 6174
+
+#ifdef FOO
+#define FOO_BRANCH(X) (X) + 1
+inline int foo(int x) {
+ if (x == FORTYTWO)
+ return FOO_BRANCH(x);
+ return FORTYTWO;
+}
+#else
+#define BAR_BRANCH(X) (X) + 2
+inline int bar(int x) {
+ if (x == FORTYTWO)
+ return BAR_BRANCH(x);
+ return FORTYTWO;
+}
+#endif
+
+// CHECK-IMP: remark: importing module './hu-01.h' from 'hu-01.pcm'
+// CHECK-HU2: ====== C++20 Module structure ======
+// CHECK-HU2-NEXT: Header Unit './hu-02.h' is the Primary Module at index #2
+// CHECK-HU2-NEXT: Exports:
+// CHECK-HU2-NEXT: Header Unit './hu-01.h' is at index #1
+// expected-no-diagnostics
+
+//--- importer-01.cpp
+export module B;
+import "hu-02.h";
+
+int success(int x) {
+ return foo(FORTYTWO + x + KAP);
+}
+
+int fail(int x) {
+ return bar(FORTYTWO + x + KAP); // expected-error {{use of undeclared identifier 'bar'}}
+ // expected-note@* {{'baz' declared here}}
+}
+
+//--- importer-02.cpp
+export module C;
+import "hu-02.h";
+
+int success(int x) {
+ return foo(FORTYTWO + x + KAP);
+}
+
+// CHECK-IMP-HU2: remark: importing module './hu-02.h' from 'hu-02.pcm'
+// CHECK-IMP-HU2: remark: importing module './hu-01.h' into './hu-02.h' from '[[TDIR]]/hu-01.pcm'
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2279,7 +2279,8 @@
/// Writes the block containing the serialized form of the
/// preprocessor.
-void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
+void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule,
+ Module *Mod) {
uint64_t MacroOffsetsBase = Stream.GetCurrentBitNo();
PreprocessingRecord *PPRec = PP.getPreprocessingRecord();
@@ -2348,13 +2349,15 @@
uint64_t StartOffset = Stream.GetCurrentBitNo() - MacroOffsetsBase;
assert((StartOffset >> 32) == 0 && "Macro identifiers offset too large");
- // Emit the macro directives in reverse source order.
- for (; MD; MD = MD->getPrevious()) {
- // Once we hit an ignored macro, we're done: the rest of the chain
- // will all be ignored macros.
- if (shouldIgnoreMacro(MD, IsModule, PP))
- break;
-
+ // Write out any exported module macros.
+ bool EmittedModuleMacros = false;
+ if (IsHeaderUnit) {
+ // This is for the main TU when it is a C++20 header unit.
+ // We preserve the final state of defined macros, and we do not emit ones
+ // that are undefined.
+ if (!MD || shouldIgnoreMacro(MD, IsModule, PP) ||
+ MD->getKind() == MacroDirective::MD_Undefine)
+ continue;
AddSourceLocation(MD->getLocation(), Record);
Record.push_back(MD->getKind());
if (auto *DefMD = dyn_cast<DefMacroDirective>(MD)) {
@@ -2362,35 +2365,51 @@
} else if (auto *VisMD = dyn_cast<VisibilityMacroDirective>(MD)) {
Record.push_back(VisMD->isPublic());
}
- }
+ ModuleMacroRecord.push_back(getSubmoduleID(Mod));
+ ModuleMacroRecord.push_back(getMacroRef(MD->getMacroInfo(), Name));
+ Stream.EmitRecord(PP_MODULE_MACRO, ModuleMacroRecord);
+ ModuleMacroRecord.clear();
+ EmittedModuleMacros = true;
+ } else {
+ // Emit the macro directives in reverse source order.
+ for (; MD; MD = MD->getPrevious()) {
+ // Once we hit an ignored macro, we're done: the rest of the chain
+ // will all be ignored macros.
+ if (shouldIgnoreMacro(MD, IsModule, PP))
+ break;
+ AddSourceLocation(MD->getLocation(), Record);
+ Record.push_back(MD->getKind());
+ if (auto *DefMD = dyn_cast<DefMacroDirective>(MD)) {
+ Record.push_back(getMacroRef(DefMD->getInfo(), Name));
+ } else if (auto *VisMD = dyn_cast<VisibilityMacroDirective>(MD)) {
+ Record.push_back(VisMD->isPublic());
+ }
+ }
- // Write out any exported module macros.
- bool EmittedModuleMacros = false;
- // We write out exported module macros for PCH as well.
- auto Leafs = PP.getLeafModuleMacros(Name);
- SmallVector<ModuleMacro*, 8> Worklist(Leafs.begin(), Leafs.end());
- llvm::DenseMap<ModuleMacro*, unsigned> Visits;
- while (!Worklist.empty()) {
- auto *Macro = Worklist.pop_back_val();
+ // We write out exported module macros for PCH as well.
+ auto Leafs = PP.getLeafModuleMacros(Name);
+ SmallVector<ModuleMacro *, 8> Worklist(Leafs.begin(), Leafs.end());
+ llvm::DenseMap<ModuleMacro *, unsigned> Visits;
+ while (!Worklist.empty()) {
+ auto *Macro = Worklist.pop_back_val();
- // Emit a record indicating this submodule exports this macro.
- ModuleMacroRecord.push_back(
- getSubmoduleID(Macro->getOwningModule()));
- ModuleMacroRecord.push_back(getMacroRef(Macro->getMacroInfo(), Name));
- for (auto *M : Macro->overrides())
- ModuleMacroRecord.push_back(getSubmoduleID(M->getOwningModule()));
+ // Emit a record indicating this submodule exports this macro.
+ ModuleMacroRecord.push_back(getSubmoduleID(Macro->getOwningModule()));
+ ModuleMacroRecord.push_back(getMacroRef(Macro->getMacroInfo(), Name));
+ for (auto *M : Macro->overrides())
+ ModuleMacroRecord.push_back(getSubmoduleID(M->getOwningModule()));
- Stream.EmitRecord(PP_MODULE_MACRO, ModuleMacroRecord);
- ModuleMacroRecord.clear();
+ Stream.EmitRecord(PP_MODULE_MACRO, ModuleMacroRecord);
+ ModuleMacroRecord.clear();
- // Enqueue overridden macros once we've visited all their ancestors.
- for (auto *M : Macro->overrides())
- if (++Visits[M] == M->getNumOverridingMacros())
- Worklist.push_back(M);
+ // Enqueue overridden macros once we've visited all their ancestors.
+ for (auto *M : Macro->overrides())
+ if (++Visits[M] == M->getNumOverridingMacros())
+ Worklist.push_back(M);
- EmittedModuleMacros = true;
+ EmittedModuleMacros = true;
+ }
}
-
if (Record.empty() && !EmittedModuleMacros)
continue;
@@ -4490,6 +4509,7 @@
using namespace llvm;
bool isModule = WritingModule != nullptr;
+ IsHeaderUnit = isModule && WritingModule->Kind == Module::ModuleHeaderUnit;
// Make sure that the AST reader knows to finalize itself.
if (Chain)
@@ -4859,7 +4879,7 @@
WriteFileDeclIDsMap();
WriteSourceManagerBlock(Context.getSourceManager(), PP);
WriteComments();
- WritePreprocessor(PP, isModule);
+ WritePreprocessor(PP, isModule, WritingModule);
WriteHeaderSearch(PP.getHeaderSearchInfo());
WriteSelectors(SemaRef);
WriteReferencedSelectorsPool(SemaRef);
Index: clang/include/clang/Serialization/ASTWriter.h
===================================================================
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -124,6 +124,9 @@
/// The module we're currently writing, if any.
Module *WritingModule = nullptr;
+ /// The module is a header unit.
+ bool IsHeaderUnit = false;
+
/// The offset of the first bit inside the AST_BLOCK.
uint64_t ASTBlockStartOffset = 0;
@@ -466,7 +469,7 @@
void WriteSourceManagerBlock(SourceManager &SourceMgr,
const Preprocessor &PP);
void writeIncludedFiles(raw_ostream &Out, const Preprocessor &PP);
- void WritePreprocessor(const Preprocessor &PP, bool IsModule);
+ void WritePreprocessor(const Preprocessor &PP, bool IsModule, Module *Mod);
void WriteHeaderSearch(const HeaderSearch &HS);
void WritePreprocessorDetail(PreprocessingRecord &PPRec,
uint64_t MacroOffsetsBase);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
