[PATCH] D119823: [Modules] Add module structure output to -module-file-info.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.


================
Comment at: clang/test/Modules/module-file-info-cxx20.cpp:26
+
+#if TU == 1
+
----------------
iains wrote:
> ChuanqiXu wrote:
> > According to [[ http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1857r3.html | P1857R3 ]], it might not be good to add macro declarative before module declaration. Although clang didn't implement it and there are old test case uses this style, I think it might be better to split into files. @urnathan how do you think about this?
> I see this in the Tony tables, but OTOH I cannot exactly see where the wording forbids it - in fact it permits preprocesssor directives before the import (including in the example in 5.7)... perhaps I'm misreading.
> 
> This way of writing the test cases does make them much easier to manage (and for a reader to see the intent) - of course, if we should split into files - then that is what we should do.
> 
I think the grammar in [[ http://eel.is/c++draft/cpp | [cpp.pre] ]] forbid it.

First, the file could only be:
```
preprocessing-file:
group_opt
module-file
```

Then it is clear that we couldn't put macro declarative before module declaration. I agree the current style is easier and more convenient. In fact, I prefer it too. But I think we would better follow it since it is standard. Otherwise, it would be more work in later maintenance stage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119823