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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
