https://github.com/ChuanqiXu9 created https://github.com/llvm/llvm-project/pull/208393
Close https://github.com/llvm/llvm-project/issues/207581 The root cause of the problem is that：enum constant decl, for which its parent is not enum class, is special. They can be accessed directly without access its parent. When modules join the game, it becomes more complex. As for members in other entities like class, we can assume the member is accessable if their parent are accesable. But it is a different story for enums. See https://github.com/llvm/llvm-project/issues/131058 for the whole story of the backrgound. Then we didn't write enum constant decl to the lookup table of its parent of parent in the ASTWriter. So that if other consumer in Sema wants to access them, they have to get it by entities like exported using decls. However, the problem is, in ASTReader, when we merge decls, we use noload_lookup to find existing decls. And the absense of unnamed enum decl in the parent of its parent's lookup table makes the merge fails. For enum constant decl in named enum, they will still have a name lookup table. So we will merge them somehow. But for enum constants in unnamed enum, they were removed from the only lookup table so the merge fails. This is the whole story of the current issue. To fix this, when we merge decls, we step into the UsingShadowDecl to find the real decls. This is fine as we won't never merge decls with different kinds. We can even call this a tiny optimization. >From 133e6094a5c08639a4752fe2a4737432f9c973f8 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <[email protected]> Date: Thu, 9 Jul 2026 15:58:43 +0800 Subject: [PATCH] [clang] [serialization] Step into UsingShadowDecl when find existing decl MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Close https://github.com/llvm/llvm-project/issues/207581 The root cause of the problem is that：enum constant decl, for which its parent is not enum class, is special. They can be accessed directly without access its parent. When modules join the game, it becomes more complex. As for members in other entities like class, we can assume the member is accessable if their parent are accesable. But it is a different story for enums. See https://github.com/llvm/llvm-project/issues/131058 for the whole story of the backrgound. Then we didn't write enum constant decl to the lookup table of its parent of parent in the ASTWriter. So that if other consumer in Sema wants to access them, they have to get it by entities like exported using decls. However, the problem is, in ASTReader, when we merge decls, we use noload_lookup to find existing decls. And the absense of unnamed enum decl in the parent of its parent's lookup table makes the merge fails. For enum constant decl in named enum, they will still have a name lookup table. So we will merge them somehow. But for enum constants in unnamed enum, they were removed from the only lookup table so the merge fails. This is the whole story of the current issue. To fix this, when we merge decls, we step into the UsingShadowDecl to find the real decls. This is fine as we won't never merge decls with different kinds. We can even call this a tiny optimization. --- clang/lib/Serialization/ASTReaderDecl.cpp | 26 ++++++++++--- clang/test/Modules/GH207581.cpp | 45 +++++++++++++++++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 clang/test/Modules/GH207581.cpp diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 370a6970d531a..bd684b89e194f 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -3415,10 +3415,20 @@ ASTDeclReader::FindExistingResult::~FindExistingResult() { } /// Find the declaration that should be merged into, given the declaration found -/// by name lookup. If we're merging an anonymous declaration within a typedef, -/// we need a matching typedef, and we merge with the type inside it. +/// by name lookup. If we're not merging with a UsingShadowDecl but Found is a +/// UsingShadowDecl, we need to skip the UsingShadowDecl. If we're merging an +/// anonymous declaration within a typedef, we need a matching typedef, and we +/// merge with the type inside it. static NamedDecl *getDeclForMerging(NamedDecl *Found, - bool IsTypedefNameForLinkage) { + bool IsTypedefNameForLinkage, + bool FilteringUsingShadowDecl) { + // If the taregt declaration we want is not a UsingShadowDecl, we don't need + // to return the UsingShadowDecl at all. + if (auto *USD = dyn_cast<UsingShadowDecl>(Found); + USD && FilteringUsingShadowDecl) + return getDeclForMerging(USD->getTargetDecl(), IsTypedefNameForLinkage, + FilteringUsingShadowDecl); + if (!IsTypedefNameForLinkage) return Found; @@ -3579,7 +3589,9 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { for (IdentifierResolver::iterator I = IdResolver.begin(Name), IEnd = IdResolver.end(); I != IEnd; ++I) { - if (NamedDecl *Existing = getDeclForMerging(*I, TypedefNameForLinkage)) + if (NamedDecl *Existing = + getDeclForMerging(*I, TypedefNameForLinkage, + /*FilteringUsingShadowDecl=*/false)) if (C.isSameEntity(Existing, D)) return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber, TypedefNameForLinkage); @@ -3587,10 +3599,12 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { } else if (DeclContext *MergeDC = getPrimaryContextForMerging(Reader, DC)) { DeclContext::lookup_result R = MergeDC->noload_lookup(Name); for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) { - if (NamedDecl *Existing = getDeclForMerging(*I, TypedefNameForLinkage)) - if (C.isSameEntity(Existing, D)) + if (NamedDecl *Existing = getDeclForMerging(*I, TypedefNameForLinkage, + !isa<UsingShadowDecl>(D))) + if (C.isSameEntity(Existing, D)) { return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber, TypedefNameForLinkage); + } } } else { // Not in a mergeable context. diff --git a/clang/test/Modules/GH207581.cpp b/clang/test/Modules/GH207581.cpp new file mode 100644 index 0000000000000..4b7c5659dd42a --- /dev/null +++ b/clang/test/Modules/GH207581.cpp @@ -0,0 +1,45 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -emit-module-interface %t/fmt.cppm -o %t/fmt.pcm +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -emit-module-interface %t/seastar.cppm -o %t/seastar.pcm +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -fmodule-file=fmt=%t/fmt.pcm -fmodule-file=seastar=%t/seastar.pcm -fsyntax-only -verify %t/main.cc + +//--- base.h +#ifndef BASE_H +#define BASE_H +namespace demo { +enum { max_packed_args = 6 }; +template <bool B, class T, class F> struct cond { using type = F; }; +template <class T, class F> struct cond<true, T, F> { using type = T; }; +template <class T> struct a_t {}; +template <class T> struct b_t {}; +template <class Context, int N> +using arg_t = typename cond<(N <= max_packed_args), a_t<Context>, b_t<Context>>::type; +} +#endif + +//--- fmt.cppm +module; +#include "base.h" +export module fmt; +export namespace demo { + using demo::max_packed_args; +} + +//--- seastar.cppm +module; +#include "base.h" +export module seastar; +export namespace seastar { inline int dummy = 0; } + +//--- main.cc +// expected-no-diagnostics +import seastar; +import fmt; +#include "base.h" +int main() { + demo::arg_t<int, 5> a; + demo::arg_t<int, 7> b; + return 0; +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
