iains updated this revision to Diff 442942.
iains marked 3 inline comments as done.
iains added a comment.
rebased, reworked
- to follow the changes proposed by core
- to make the diagnostics follow that and a compromise for the proposed
revision before the core amendment.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128328/new/
https://reviews.llvm.org/D128328
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/Sema/SemaModule.cpp
clang/test/Modules/Reachability-Private.cpp
clang/test/Modules/cxx20-10-5-ex1.cpp
Index: clang/test/Modules/cxx20-10-5-ex1.cpp
===================================================================
--- /dev/null
+++ 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 {{exported inline function not defined before the private module fragment}}
+ // expected-n...@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 {{un-exported inline function not defined before the private module fragment}}
+ // expected-n...@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-n...@std-10-5-ex1-interface.cpp:22 1+{{definition here is not reachable}}
+ X *p = factory();
+}
Index: clang/test/Modules/Reachability-Private.cpp
===================================================================
--- clang/test/Modules/Reachability-Private.cpp
+++ 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-n...@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 @@
//--- 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-n...@private.cppm:18 1+{{definition here is not reachable}}
auto _ = factory();
auto *__ = factory();
X *___ = factory();
Index: clang/lib/Sema/SemaModule.cpp
===================================================================
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -901,6 +901,17 @@
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);
+ }
}
}
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -5643,7 +5643,7 @@
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);
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -9690,6 +9690,19 @@
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.
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1252,6 +1252,32 @@
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 IsExport = FD->isInExportDeclContext();
+ bool DefInPMF = false;
+ if (auto *FDD = FD->getDefinition()) {
+ DefInPMF = FDD->getOwningModule()->isPrivateModule();
+ if (!DefInPMF)
+ continue;
+ }
+ Diag(FD->getLocation(), diag::err_export_inline_not_defined)
+ << IsExport << 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
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2252,6 +2252,11 @@
/// 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 Decl *, 8> PendingInlineFuncDecls;
+
/// Helper function to judge if we are in module purview.
/// Return false if we are not in a module.
bool isCurrentModulePurview() const {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11154,6 +11154,9 @@
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<
+ "%select{un-|}0exported inline function not defined"
+ "%select{| before the private module fragment}1">;
def err_export_partition_impl : Error<
"module partition implementations cannot be exported">;
def err_export_in_private_module_fragment : Error<
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
