[llvm-branch-commits] [clang] release/22.x: [C++20] [Modules] Add VisiblePromoted module ownership kind (#189903) (PR #192885)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Apr 19 19:19:27 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-modules
Author: llvmbot
<details>
<summary>Changes</summary>
Backport c97e08e331736ae8c7d17bf1f24954570f564ad0
Requested by: @<!-- -->ChuanqiXu9
---
Full diff: https://github.com/llvm/llvm-project/pull/192885.diff
7 Files Affected:
- (modified) clang/include/clang/AST/DeclBase.h (+14-1)
- (modified) clang/include/clang/AST/DeclContextInternals.h (+3-1)
- (modified) clang/lib/AST/Decl.cpp (+1)
- (modified) clang/lib/AST/DeclBase.cpp (+2-1)
- (modified) clang/lib/Sema/SemaLookup.cpp (+1-1)
- (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+1)
- (added) clang/test/Modules/include-between-imports-enums.cppm (+47)
``````````diff
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 66c625f41158a..231fc381bda8e 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 b9fac5a4a1153..7c5124a38d811 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 f8e9caa3f5d1d..45ba4231204c5 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;
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/192885
More information about the llvm-branch-commits
mailing list