ChuanqiXu added inline comments.

================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11155
+def err_export_inline_not_defined : Error<
+  "exported inline functions must be defined within the module purview"
+  " and before any private module fragment">;
----------------
iains wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > iains wrote:
> > > > ChuanqiXu wrote:
> > > > > iains wrote:
> > > > > > iains wrote:
> > > > > > > ChuanqiXu wrote:
> > > > > > > > From my reading, 'exported' is not emphasized.
> > > > > > > it is here:
> > > > > > > https://eel.is/c++draft/module#private.frag-2.1
> > > > > > > ( I agree it is somewhat confusing, but the export makes the 
> > > > > > > linkage external, which the example treats differently from the 
> > > > > > > fn_m() case which has module linkage).
> > > > > > > 
> > > > > > > It is possible that we might need to pull together several pieces 
> > > > > > > of the std and maybe ask core for clarification?
> > > > > > > it is here:
> > > > > > > https://eel.is/c++draft/module#private.frag-2.1
> > > > > > > ( I agree it is somewhat confusing, but the export makes the 
> > > > > > > linkage external, which the example treats differently from the 
> > > > > > > fn_m() case which has module linkage).
> > > > > > 
> > > > > > hmm... my linkage comment is wrong - however the distinction 
> > > > > > between exported and odr-used seems to be made here (fn_m() and 
> > > > > > fn_e()).
> > > > > > > 
> > > > > > > It is possible that we might need to pull together several pieces 
> > > > > > > of the std and maybe ask core for clarification?
> > > > > > 
> > > > > > 
> > > > > What I read is:
> > > > > > [dcl.inline]p7: https://eel.is/c++draft/dcl.inline#7
> > > > > >     If an inline function or variable that is attached to a named 
> > > > > > module is declared in a definition domain, it shall be defined in 
> > > > > > that domain.
> > > > > 
> > > > > and the definition of `definition domain` is:
> > > > > > [basic.def.odr]p12: https://eel.is/c++draft/basic#def.odr-12
> > > > > >       A definition domain is a private-module-fragment or the 
> > > > > > portion of a translation unit excluding its private-module-fragment 
> > > > > > (if any).
> > > > > 
> > > > > The definition of "attached to a named module" is:
> > > > > > [module.unit]p7: https://eel.is/c++draft/module.unit#7
> > > > > >      A module is either a named module or the global module. A 
> > > > > > declaration is attached to a module as follows: ...
> > > > > 
> > > > > So it is clearly not consistency with [module.private.frag]p2.1. I 
> > > > > would send this to WG21.
> > > > Yes, that was what I found - maybe we are  missing something about the 
> > > > export that changes those rules.
> > > > .
> > > I think that we can consider this closed by the question to the ext 
> > > reflector and the amendment proposed by core.
> > I prefer `inline function attached to a named module not defined %select{| 
> > before the private module fragment}1`. Since the `export` part is not 
> > important here and the important part is whether or not they are attached 
> > to a named module.
> I took the error message from here:
> https://github.com/cplusplus/draft/pull/5537
> which was prepared after the dicussion that you started on the ext reflector. 
>  Actually, I do not have a strong feeling either way.
Oh, I didn't track that. I feel my suggested version is slightly better. 
Otherwise users might want to make it work by adding/removing `export` clause.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128328

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to