iains marked 3 inline comments as done.
iains added inline comments.
================
Comment at: clang/include/clang/Lex/Preprocessor.h:434
+ /// Saw a 'module' identifier.
+ void handleModule(bool ATLTS) {
+ // This was the first module identifier and not preceded by any token
----------------
ChuanqiXu wrote:
> What's the meaning of `ATLTS`?
`AfterTopLevelTokenSeq` I guess it seemed a rather long name (but I do not mind
to spell it out if that makes sense).
================
Comment at: clang/lib/Lex/PPDirectives.cpp:2226-2227
+
+ // FIXME: We do not have a good way to disambiguate C++ clang modules from
+ // C++ standard modules (other than use/non-use of Header Units).
+ Module *SM = SuggestedModule.getModule();
----------------
ChuanqiXu wrote:
> From what @rsmith said in https://reviews.llvm.org/D113391, it looks like the
> ideal direction is to use C++ clang modules and C++ standard modules
> together. So it looks like we couldn't disambiguate them from command line
> options.
Well, I think there is some more debate to have around how to solve this
problem (i.e. it might be intended that clang++ modules and standard c++
modules converge but as things stand we still need to support the cases that
they have different behaviour, or break existing users)
... - but please let us not have that debate in this patch :-)
================
Comment at: clang/test/Modules/cxx20-include-translation.cpp:10
+// RUN: %clang_cc1 -std=c++20 -xc++-user-header h4.h -emit-header-unit -o
h4.pcm
+
+// RUN: %clang_cc1 -std=c++20 Xlate.cpp -emit-module-interface -o Xlate.pcm \
----------------
ChuanqiXu wrote:
> Maybe we need an example `h5.h` which is not pre-compiled as a header unit
> and it would be handled correctly.
I am not sure if you mean that the un-converted header would be included in the
GMF or in the module purview - IMO we already have test-cases that cover this,
but I do not mind adding something extra if there's a missing case?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128981/new/
https://reviews.llvm.org/D128981
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
