[PATCH] D128328: [C++20][Modules] Improve handing of Private Module Fragment diagnostics.

[Iain Sandoe via Phabricator via cfe-commits]

iains marked 2 inline comments as done.
iains added a comment.

In D128328#3601080 <https://reviews.llvm.org/D128328#3601080>, @ChuanqiXu wrote:

> It looks like we need to handle inline variable as well to match the intention.

can you construct a test-case, where this would apply and which is not already diagnosed as incorrect?



================
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:
> 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?


================
Comment at: clang/lib/Sema/SemaModule.cpp:895-905
+      if (auto *FD = dyn_cast<FunctionDecl>(Child)) {
+        // [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.
+        // So, if the current declaration does not have a definition, we must
+        // check at the end of the TU (or when the PMF starts) to see that we
----------------
ChuanqiXu wrote:
> So we might need to move this to somewhere else.
depending on reading of the cases that can have this effect.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128328