Author: Iain Sandoe Date: 2022-08-21T10:19:46+01:00 New Revision: fee3cccc6cdabdeddc688cf7a15b144c814dd93d
URL: https://github.com/llvm/llvm-project/commit/fee3cccc6cdabdeddc688cf7a15b144c814dd93d DIFF: https://github.com/llvm/llvm-project/commit/fee3cccc6cdabdeddc688cf7a15b144c814dd93d.diff LOG: [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 Added: clang/test/Modules/cxx20-10-5-ex1.cpp Modified: 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 Removed: ################################################################################ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index e4e7d7b7338a4..791ef9d71dec8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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< diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 28a47a2eea8b8..14bb2b1d2beed 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2258,6 +2258,11 @@ class Sema final { /// 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 { diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 1d27f74f00e56..04100324c786e 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/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 diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 33b49482c6e55..004c1ffd81b9f 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/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. diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp index 1d8df945a4fe2..22d72c3251a0c 100644 --- a/clang/lib/Sema/SemaLookup.cpp +++ b/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); diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp index 757c611d61ba9..b205fd914f9d3 100644 --- a/clang/lib/Sema/SemaModule.cpp +++ b/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); + } } } diff --git a/clang/test/Modules/Reachability-Private.cpp b/clang/test/Modules/Reachability-Private.cpp index fdaf5c349f8ae..9a7c3ba231f17 100644 --- a/clang/test/Modules/Reachability-Private.cpp +++ b/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 @@ 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-n...@private.cppm:18 1+{{definition here is not reachable}} auto _ = factory(); auto *__ = factory(); X *___ = factory(); diff --git a/clang/test/Modules/cxx20-10-5-ex1.cpp b/clang/test/Modules/cxx20-10-5-ex1.cpp new file mode 100644 index 0000000000000..c24fb5fe1fea5 --- /dev/null +++ b/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-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 {{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(); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
