iains marked an inline comment as done. iains 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">; ---------------- ChuanqiXu wrote: > 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. I removed the reference to un/exported. 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
