iains marked 4 inline comments as done. iains added inline comments.
================ Comment at: clang/include/clang/Basic/Module.h:109 - /// This is a C++ Modules TS module interface unit. + /// This is a C++ Modules TS or C++20 module interface unit. ModuleInterfaceUnit, ---------------- urnathan wrote: > iains wrote: > > urnathan wrote: > > > I think it's confusing to continue to refer to modules-ts modules at this > > > point. Is that really necessary? > > > The specific confusion here is that does 'ModuleInterfaceUnit' mean one > > > of two different things depending on compilation mode, or is it a single > > > thing with two different names? > > In this case, I re-used the enumeration since the modules-ts / c++20 > > modules are disambiguated by the -fmodules-ts flag. > > > > So, yes, the enum value does have two uses depending on compilation mode. > > I can amend the comment to try and make this clear, would that help? > > > > I have been trying not to regress anything for modules-ts (and certainly > > for clang modules) - if someone makes a decision to retire modules-ts then > > there is probably a bunch of simplification that could be made. > > > > With the extra bit to represent the enumeration, we have more space so we > > *could* have different names for the modules-ts/C++20 interfaces (although > > I suspect that will just make more code at the use sites, and would prefer > > not to go down that route). > > > IMHO 'modules-ts' is not a useful thing distinct from 'c++20 modules'. The > semantics of the TS itself were unclear or broken in places, and only > resolved once things got into the std (via 1103/Atom). I think the semantics > of '-fmodules-ts' should be the module-specific semantics of C++20 as applied > to whatever version of the std being selected (and I'd be fine making it > Buyer-Beware for anything before C++20 anyway). > > But, if you don't want to bite that bullet right now, a clarifying note would > be helpful. > I think we can just go with C++20 and as you suggest treat the TS as a dialect. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114714/new/ https://reviews.llvm.org/D114714 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits