https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/189903
This patch adds a new ModuleOwnershipKind::VisiblePromoted to handle declarations that are not visible to the current TU but are promoted to be visible to avoid re-parsing. Originally we set the visible visiblity directly in such cases. But https://github.com/llvm/llvm-project/issues/188853 shows such decls may be excluded later if we import #include and then import. So we have to introduce a new visibility to express the intention that the visibility of the decl is intentionally promoted. Close https://github.com/llvm/llvm-project/issues/188853 >From 0d78f704c3cfe6e7cdeeecfa83124fcd4135c7dc Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <[email protected]> Date: Wed, 1 Apr 2026 15:49:37 +0800 Subject: [PATCH] [C++20] [Modules] Add VisiblePromoted module ownership kind This patch adds a new ModuleOwnershipKind::VisiblePromoted to handle declarations that are not visible to the current TU but are promoted to be visible to avoid re-parsing. Originally we set the visible visiblity directly in such cases. But https://github.com/llvm/llvm-project/issues/188853 shows such decls may be excluded later if we import #include and then import. So we have to introduce a new visibility to express the intention that the visibility of the decl is intentionally promoted. Close https://github.com/llvm/llvm-project/issues/188853 --- clang/include/clang/AST/DeclBase.h | 15 +++++- .../include/clang/AST/DeclContextInternals.h | 4 +- clang/lib/AST/Decl.cpp | 1 + clang/lib/AST/DeclBase.cpp | 3 +- clang/lib/Sema/SemaLookup.cpp | 2 +- clang/lib/Serialization/ASTReaderDecl.cpp | 1 + .../include-between-imports-enums.cppm | 47 +++++++++++++++++++ 7 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 clang/test/Modules/include-between-imports-enums.cppm diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 5519787d71f88..3c5a093439ceb 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -228,6 +228,14 @@ class alignas(8) Decl { /// module is imported. VisibleWhenImported, + /// This declaration has an owning module, and is not visible to the + /// current TU but we promoted it to be visible for various reasons, + /// e.g., we have the same declaration in the current TU but we'd + /// like to avoid parsing it again. + /// + /// The vibility should be never be serialized. + VisiblePromoted, + /// This declaration has an owning module, and is visible to lookups /// that occurs within that module. And it is reachable in other module /// when the owning module is transitively imported. @@ -668,7 +676,7 @@ class alignas(8) Decl { bool isInExportDeclContext() const; bool isInvisibleOutsideTheOwningModule() const { - return getModuleOwnershipKind() > ModuleOwnershipKind::VisibleWhenImported; + return getModuleOwnershipKind() > ModuleOwnershipKind::VisiblePromoted; } /// Whether this declaration comes from another module unit. @@ -872,6 +880,11 @@ class alignas(8) Decl { setModuleOwnershipKind(ModuleOwnershipKind::Visible); } + void setVisiblePromoted() { + if (isInvisibleOutsideTheOwningModule() && isFromASTFile()) + setModuleOwnershipKind(ModuleOwnershipKind::VisiblePromoted); + } + /// Get the kind of module ownership for this declaration. ModuleOwnershipKind getModuleOwnershipKind() const { return NextInContextAndBits.getInt(); diff --git a/clang/include/clang/AST/DeclContextInternals.h b/clang/include/clang/AST/DeclContextInternals.h index a0f37886cc014..d87d8e8a663fa 100644 --- a/clang/include/clang/AST/DeclContextInternals.h +++ b/clang/include/clang/AST/DeclContextInternals.h @@ -174,7 +174,9 @@ class StoredDeclsList { // Remove all declarations that are either external or are replaced with // external declarations with higher visibilities. DeclListNode::Decls *Tail = erase_if([Decls](NamedDecl *ND) { - if (ND->isFromASTFile()) + // If the declaration is promoted intentionally, keep it. + if (ND->isFromASTFile() && ND->getModuleOwnershipKind() != + Decl::ModuleOwnershipKind::VisiblePromoted) return true; // FIXME: Can we get rid of this loop completely? return llvm::any_of(Decls, [ND](NamedDecl *D) { diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 37b00eeca539c..87d5a1f3c524b 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -1193,6 +1193,7 @@ static bool isExportedFromModuleInterfaceUnit(const NamedDecl *D) { case Decl::ModuleOwnershipKind::Unowned: case Decl::ModuleOwnershipKind::ReachableWhenImported: case Decl::ModuleOwnershipKind::ModulePrivate: + case Decl::ModuleOwnershipKind::VisiblePromoted: return false; case Decl::ModuleOwnershipKind::Visible: case Decl::ModuleOwnershipKind::VisibleWhenImported: diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 0a1e442656c35..18b6a5b06bab8 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -402,7 +402,8 @@ void Decl::setLexicalDeclContext(DeclContext *DC) { } assert( - (getModuleOwnershipKind() != ModuleOwnershipKind::VisibleWhenImported || + ((getModuleOwnershipKind() != ModuleOwnershipKind::VisibleWhenImported && + getModuleOwnershipKind() != ModuleOwnershipKind::VisiblePromoted) || getOwningModule()) && "hidden declaration has no owning module"); } diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp index de53f6010a1b6..b96065f8619d2 100644 --- a/clang/lib/Sema/SemaLookup.cpp +++ b/clang/lib/Sema/SemaLookup.cpp @@ -1575,7 +1575,7 @@ void Sema::makeMergedDefinitionVisible(NamedDecl *ND) { if (auto *ED = dyn_cast<EnumDecl>(ND); ED && ED->isFromGlobalModule() && !ED->isScoped()) { for (auto *ECD : ED->enumerators()) { - ECD->setVisibleDespiteOwningModule(); + ECD->setVisiblePromoted(); DeclContext *RedeclCtx = ED->getDeclContext()->getRedeclContext(); if (RedeclCtx->lookup(ECD->getDeclName()).empty()) RedeclCtx->makeDeclVisibleInContext(ECD); diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index b49bd5ea8bca6..9033ea55bc5e2 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -636,6 +636,7 @@ void ASTDeclReader::VisitDecl(Decl *D) { break; case Decl::ModuleOwnershipKind::Unowned: case Decl::ModuleOwnershipKind::VisibleWhenImported: + case Decl::ModuleOwnershipKind::VisiblePromoted: case Decl::ModuleOwnershipKind::ReachableWhenImported: case Decl::ModuleOwnershipKind::ModulePrivate: break; diff --git a/clang/test/Modules/include-between-imports-enums.cppm b/clang/test/Modules/include-between-imports-enums.cppm new file mode 100644 index 0000000000000..547848e4057a9 --- /dev/null +++ b/clang/test/Modules/include-between-imports-enums.cppm @@ -0,0 +1,47 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-module-interface -o %t/B.pcm +// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fprebuilt-module-path=%t -verify -fsyntax-only +// +// RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-reduced-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-reduced-module-interface -o %t/B.pcm +// RUN: %clang_cc1 -std=c++20 %t/use.cpp -fprebuilt-module-path=%t -verify -fsyntax-only + +// Test that a textual #include sandwiched between two import declarations +// of modules that both include the same header in their GMFs does not lose +// enum declarations. See https://github.com/llvm/llvm-project/issues/188853 + +//--- enum.h +#ifndef ENUM_H +#define ENUM_H +namespace ns { +enum E { Value1, Value2, Value3 }; +} +#endif + +//--- A.cppm +module; +#include "enum.h" +export module A; +export auto a = ns::Value1; + +//--- B.cppm +module; +#include "enum.h" +export module B; +export auto b = ns::Value2; + +//--- use.cpp +// expected-no-diagnostics +import A; +#include "enum.h" +import B; + +auto x = ns::Value3; + +namespace ns { +auto y = Value1; +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
