[clang] [C++20] [Modules] Add VisiblePromoted module ownership kind (PR #189903)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 1 00:56:44 PDT 2026
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 <yedeng.yd at linux.alibaba.com>
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;
+}
More information about the cfe-commits
mailing list