Author: Chuanqi Xu Date: 2024-04-30T11:34:34+08:00 New Revision: 38067c50a9459caed2892e38b2ae5026a8bff8e2
URL: https://github.com/llvm/llvm-project/commit/38067c50a9459caed2892e38b2ae5026a8bff8e2 DIFF: https://github.com/llvm/llvm-project/commit/38067c50a9459caed2892e38b2ae5026a8bff8e2.diff LOG: [C++20] [Modules] [Reduced BMI] Avoid force writing static declarations within module purview Close https://github.com/llvm/llvm-project/issues/90259 Technically, the static declarations shouldn't be leaked from the module interface, otherwise it is an illegal program according to the spec. So we can get rid of the static declarations from the reduced BMI technically. Then we can close the above issue. However, there are too many `static inline` codes in existing headers. So it will be a pretty big breaking change if we do this globally. Added: clang/test/Modules/pr90259.cppm Modified: clang/lib/Serialization/ASTWriter.cpp Removed: ################################################################################ diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 0408eeb6a95b00..7db60c67d71234 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -3205,6 +3205,17 @@ void ASTWriter::WriteType(QualType T) { // Declaration Serialization //===----------------------------------------------------------------------===// +static bool IsInternalDeclFromFileContext(const Decl *D) { + auto *ND = dyn_cast<NamedDecl>(D); + if (!ND) + return false; + + if (!D->getDeclContext()->getRedeclContext()->isFileContext()) + return false; + + return ND->getFormalLinkage() == Linkage::Internal; +} + /// Write the block containing all of the declaration IDs /// lexically declared within the given DeclContext. /// @@ -3225,6 +3236,15 @@ uint64_t ASTWriter::WriteDeclContextLexicalBlock(ASTContext &Context, if (DoneWritingDeclsAndTypes && !wasDeclEmitted(D)) continue; + // We don't need to write decls with internal linkage into reduced BMI. + // If such decls gets emitted due to it get used from inline functions, + // the program illegal. However, there are too many use of static inline + // functions in the global module fragment and it will be breaking change + // to forbid that. So we have to allow to emit such declarations from GMF. + if (GeneratingReducedBMI && !D->isFromExplicitGlobalModule() && + IsInternalDeclFromFileContext(D)) + continue; + KindDeclPairs.push_back(D->getKind()); KindDeclPairs.push_back(GetDeclRef(D).get()); } @@ -3886,6 +3906,13 @@ class ASTDeclContextNameLookupTrait { !Writer.wasDeclEmitted(DeclForLocalLookup)) continue; + // Try to avoid writing internal decls to reduced BMI. + // See comments in ASTWriter::WriteDeclContextLexicalBlock for details. + if (Writer.isGeneratingReducedBMI() && + !DeclForLocalLookup->isFromExplicitGlobalModule() && + IsInternalDeclFromFileContext(DeclForLocalLookup)) + continue; + DeclIDs.push_back(Writer.GetDeclRef(DeclForLocalLookup)); } return std::make_pair(Start, DeclIDs.size()); @@ -4257,6 +4284,12 @@ uint64_t ASTWriter::WriteDeclContextVisibleBlock(ASTContext &Context, if (DoneWritingDeclsAndTypes && !wasDeclEmitted(ND)) continue; + // We don't need to force emitting internal decls into reduced BMI. + // See comments in ASTWriter::WriteDeclContextLexicalBlock for details. + if (GeneratingReducedBMI && !ND->isFromExplicitGlobalModule() && + IsInternalDeclFromFileContext(ND)) + continue; + GetDeclRef(ND); } } @@ -4917,8 +4950,7 @@ void ASTWriter::PrepareWritingSpecialDecls(Sema &SemaRef) { // is ill-formed. However, in practice, there are a lot of projects // uses `static inline` in the headers. So we can't get rid of all // static entities in reduced BMI now. - if (auto *ND = dyn_cast<NamedDecl>(D); - ND && ND->getFormalLinkage() == Linkage::Internal) + if (IsInternalDeclFromFileContext(D)) continue; } diff --git a/clang/test/Modules/pr90259.cppm b/clang/test/Modules/pr90259.cppm new file mode 100644 index 00000000000000..17786998a2a729 --- /dev/null +++ b/clang/test/Modules/pr90259.cppm @@ -0,0 +1,44 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 %t/mod1.cppm -emit-reduced-module-interface -o %t/mod-mod1.pcm +// RUN: %clang_cc1 -std=c++20 %t/mod.cppm -fprebuilt-module-path=%t \ +// RUN: -emit-reduced-module-interface -o %t/mod.pcm +// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fprebuilt-module-path=%t -verify -fsyntax-only + +//--- mod1.cppm +export module mod:mod1; +namespace { + int abc = 43; +} +namespace mod { + static int def = 44; +} +export int f() { + return abc + mod::def; +} + +//--- mod.cppm +// expected-no-diagnostics +export module mod; +import :mod1; + +namespace { + double abc = 43.0; +} + +namespace mod { + static double def = 44.0; +} + +export double func() { + return (double)f() + abc + mod::def; +} + +//--- use.cpp +// expected-no-diagnostics +import mod; +double use() { + return func(); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
