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

Reply via email to