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

This adds a check for exported inline functions, that there is a definition in
the definition domain (which, in practice, can only be the module purview but
before any PMF starts) since the PMF definition domain cannot contain exports.

This is:
[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.

The patch also amends diagnostic output by excluding the PMF sub-module from the
set considered as sources of missing decls.  There is no point in telling the user
that the import of a PMF object is missing - since such objects are never reachable
to an importer.  We still show the definition (as unreachable), to help point out
this.

Differential Revision: https://reviews.llvm.org/D128328

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -11141,6 +11141,8 @@ def err_export_using_internal : Error<
 def err_export_not_in_module_interface : Error<
   "export declaration can only be used within a module interface unit"
   "%select{ after the module declaration|}0">;
+def err_export_inline_not_defined : Error<
+  "inline function not defined%select{| before the private module fragment}0">;
 def err_export_partition_impl : Error<
   "module partition implementations cannot be exported">;
 def err_export_in_private_module_fragment : Error<

clang/include/clang/Sema/Sema.h

@@ -2258,6 +2258,11 @@ private:
   /// Namespace definitions that we will export when they finish.
   llvm::SmallPtrSet<const NamespaceDecl*, 8> DeferredExportedNamespaces;
 
+  /// In a C++ standard module, inline declarations require a definition to be
+  /// present at the end of a definition domain.  This set holds the decls to
+  /// be checked at the end of the TU.
+  llvm::SmallPtrSet<const FunctionDecl *, 8> PendingInlineFuncDecls;
+
   /// Helper function to judge if we are in module purview.
   /// Return false if we are not in a module.
   bool isCurrentModulePurview() const {

clang/lib/Sema/Sema.cpp

@@ -1251,6 +1251,33 @@ void Sema::ActOnEndOfTranslationUnit() {
     emitAndClearUnusedLocalTypedefWarnings();
   }
 
+  // C++ standard modules. Diagnose cases where a function is declared inline
+  // in the module purview but has no definition before the end of the TU or
+  // the start of a Private Module Fragment (if one is present).
+  if (!PendingInlineFuncDecls.empty()) {
+    for (auto *D : PendingInlineFuncDecls) {
+      if (auto *FD = dyn_cast<FunctionDecl>(D)) {
+        bool DefInPMF = false;
+        if (auto *FDD = FD->getDefinition()) {
+          assert(FDD->getOwningModule() &&
+                 FDD->getOwningModule()->isModulePurview());
+          DefInPMF = FDD->getOwningModule()->isPrivateModule();
+          if (!DefInPMF)
+            continue;
+        }
+        Diag(FD->getLocation(), diag::err_export_inline_not_defined)
+            << DefInPMF;
+        // If we have a PMF it should be at the end of the ModuleScopes.
+        if (DefInPMF &&
+            ModuleScopes.back().Module->Kind == Module::PrivateModuleFragment) {
+          Diag(ModuleScopes.back().BeginLoc,
+               diag::note_private_module_fragment);
+        }
+      }
+    }
+    PendingInlineFuncDecls.clear();
+  }
+
   // C99 6.9.2p2:
   //   A declaration of an identifier for an object that has file
   //   scope without an initializer, and without a storage-class

clang/lib/Sema/SemaDecl.cpp

@@ -9838,6 +9838,19 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
       NewFD->setType(Context.getFunctionType(
           FPT->getReturnType(), FPT->getParamTypes(),
           FPT->getExtProtoInfo().withExceptionSpec(EST_BasicNoexcept)));
+
+    // C++20 [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
+    // have a definition at that point.
+    if (isInline && !D.isFunctionDefinition() && getLangOpts().CPlusPlus20 &&
+        NewFD->hasOwningModule() &&
+        NewFD->getOwningModule()->isModulePurview()) {
+      PendingInlineFuncDecls.insert(NewFD);
+    }
   }
 
   // Filter out previous declarations that don't match the scope.

clang/lib/Sema/SemaLookup.cpp

@@ -5691,7 +5691,7 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, NamedDecl *Decl,
   llvm::SmallVector<Module*, 8> UniqueModules;
   llvm::SmallDenseSet<Module*, 8> UniqueModuleSet;
   for (auto *M : Modules) {
-    if (M->Kind == Module::GlobalModuleFragment)
+    if (M->isGlobalModule() || M->isPrivateModule())
       continue;
     if (UniqueModuleSet.insert(M).second)
       UniqueModules.push_back(M);

clang/lib/Sema/SemaModule.cpp

@@ -910,6 +910,17 @@ Decl *Sema::ActOnFinishExportDecl(Scope *S, Decl *D, SourceLocation RBraceLoc) {
         diagExportedUnnamedDecl(*this, UnnamedDeclKind::Context, Child,
                                 BlockStart);
       }
+      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
+        // have a definition at that point.
+        if (FD->isInlineSpecified() && !FD->isDefined())
+          PendingInlineFuncDecls.insert(FD);
+      }
     }
   }
 

clang/test/Modules/Reachability-Private.cpp

@@ -4,18 +4,25 @@
 // RUN: mkdir -p %t
 // RUN: split-file %s %t
 //
-// RUN: %clang_cc1 -std=c++20 %t/Private.cppm -emit-module-interface -o %t/Private.pcm
-// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use.cpp -verify -fsyntax-only
+// RUN: %clang_cc1 -std=c++20 %t/Private.cppm -emit-module-interface \
+// RUN: -o %t/Private.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/Use.cpp \
+// RUN: -DTEST_BADINLINE -verify -fsyntax-only
 
 //--- Private.cppm
 export module Private;
-inline void fn_m(); // OK, module-linkage inline function
+#ifdef TEST_BADINLINE
+inline void fn_m(); // expected-error {{un-exported inline function not defined before the private module fragment}}
+                    // expected-note@Private.cppm:13 {{private module fragment begins here}}
+#endif
 static void fn_s();
 export struct X;
 
 export void g(X *x) {
   fn_s(); // OK, call to static function in same translation unit
-  fn_m(); // OK, call to module-linkage inline function
+#ifdef TEST_BADINLINE
+  fn_m(); // fn_m is not OK.
+#endif
 }
 export X *factory(); // OK
 
@@ -30,10 +37,8 @@ void fn_s() {}
 //--- Use.cpp
 import Private;
 void foo() {
-  X x; // expected-error {{definition of 'X' must be imported from module 'Private.<private>' before it is required}}
-       // expected-error@-1 {{definition of 'X' must be imported from module 'Private.<private>' before it is required}}
-       // expected-note@* {{definition here is not reachable}}
-       // expected-note@* {{definition here is not reachable}}
+  X x; // expected-error 1+{{missing '#include'; 'X' must be defined before it is used}}
+       // expected-note@Private.cppm:18 1+{{definition here is not reachable}}
   auto _ = factory();
   auto *__ = factory();
   X *___ = factory();

clang/test/Modules/cxx20-10-5-ex1.cpp

@@ -0,0 +1,53 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface std-10-5-ex1-interface.cpp \
+// RUN: -DBAD_FWD_DECL  -fsyntax-only -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface std-10-5-ex1-interface.cpp \
+// RUN: -o A.pcm
+
+// RUN: %clang_cc1 -std=c++20 std-10-5-ex1-use.cpp  -fmodule-file=A.pcm \
+// RUN:    -fsyntax-only -verify
+
+//--- std-10-5-ex1-interface.cpp
+
+export module A;
+#ifdef BAD_FWD_DECL
+export inline void fn_e(); // expected-error {{inline function not defined before the private module fragment}}
+                           // expected-note@std-10-5-ex1-interface.cpp:21 {{private module fragment begins here}}
+#endif
+export inline void ok_fn() {}
+export inline void ok_fn2();
+#ifdef BAD_FWD_DECL
+inline void fn_m(); // expected-error {{inline function not defined before the private module fragment}}
+                    // expected-note@std-10-5-ex1-interface.cpp:21 {{private module fragment begins here}}
+#endif
+static void fn_s();
+export struct X;
+export void g(X *x) {
+  fn_s();
+}
+export X *factory();
+void ok_fn2() {}
+
+module :private;
+struct X {};
+X *factory() {
+  return new X();
+}
+
+void fn_e() {}
+void fn_m() {}
+void fn_s() {}
+
+//--- std-10-5-ex1-use.cpp
+
+import A;
+
+void foo() {
+  X x; // expected-error 1+{{missing '#include'; 'X' must be defined before it is used}}
+       // expected-note@std-10-5-ex1-interface.cpp:22 1+{{definition here is not reachable}}
+  X *p = factory();
+}