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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

I feel good if we could add  the test from: http://eel.is/c++draft/cpp.import#8.



================
Comment at: clang/include/clang/Serialization/ASTWriter.h:21
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/Module.h"
 #include "clang/Basic/SourceLocation.h"
----------------
This is redundant too.


================
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)
----------------
iains wrote:
> 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).
> 
I agree with you. I tried to refactor it myself and find things didn't get better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121097