ChuanqiXu added a comment.
Maybe it's worth to add a test from: http://eel.is/c++draft/cpp.import#8
================
Comment at: clang/include/clang/Serialization/ASTWriter.h:127-128
+ /// The module is a header unit.
+ bool IsHeaderUnit = false;
+
----------------
I think the member is redundant. I thought we could use
`WritingModule->isHeaderUnit()` to replace it.
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:2287-2288
/// preprocessor.
-void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule) {
+void ASTWriter::WritePreprocessor(const Preprocessor &PP, bool IsModule,
+ Module *Mod) {
uint64_t MacroOffsetsBase = Stream.GetCurrentBitNo();
----------------
We could replace the use of `Mod` with WritingModule too. (If I don't misread
the code)
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:2358-2378
+ 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)
----------------
Is it possible to merge the implementation with the following for PCH? It looks
like there are some redundancies.
================
Comment at: clang/lib/Serialization/ASTWriter.cpp:2360-2362
+ // 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.
----------------
How about to add comments to show the **different logic** of emitting macros
for header unit and PCH? I guess other readers might be confusing too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121097/new/
https://reviews.llvm.org/D121097
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
