[PATCH] D121097: [C++20][Modules][HU 3/5] Emit module macros for header units.

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.


================
Comment at: clang/include/clang/Serialization/ASTWriter.h:127-128
 
+  /// The module is a header unit.
+  bool IsHeaderUnit = false;
+
----------------
ChuanqiXu wrote:
> I think the member is redundant. I thought we could use `WritingModule->isHeaderUnit()` to replace it.
yeah we can do this - I added a "isHeaderUnit()" method to Module.h to do this.


================
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)
----------------
ChuanqiXu wrote:
> Is it possible to merge the implementation with the following for PCH? It looks like there are some redundancies.
Well, that was what I had originally, I actually split it out as it is now because the difference in the logic around which macros are written out was making the code pretty confusing to read.  If there's a strong feeling about this, perhaps we can see if there's some way to factor it (perhaps with some place-holder vars).



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121097/new/

https://reviews.llvm.org/D121097