https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/214121
>From 72281ba48937660309da35dda79af85cc9bf5e9d Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <[email protected]> Date: Wed, 5 Aug 2026 10:56:28 +0800 Subject: [PATCH 1/4] [C++20] [Modules] merge the type for anony enum from import and #include Close https://github.com/llvm/llvm-project/issues/213299 Ideally, we shall merge the new enum with the old enum when we creating the new enum. But the enum is anonymous and the typedef's name come after the enum body, it is too late to merge them. This is the choice 10 years ago: a523022b5384d7a0901beea7a5f36ee9c09ba339. Actually what we're merging is the typedef decls. Then https://github.com/llvm/llvm-project/pull/114240 removes the logic to remove the new ED. This the direct trigger for the above issue of ambiguous look ups. We choose to fix the problem by setting the type of new enum to the type of the old enum to fix the ambiguous lookup issue. --- clang/docs/ReleaseNotes.md | 4 +++ clang/lib/Sema/SemaDecl.cpp | 27 +++++++++++++++++++ .../Modules/include-after-imports-enums.cppm | 8 ++++++ 3 files changed, 39 insertions(+) diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index a38b99ff8e075..209278f105ea4 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -385,6 +385,10 @@ features cannot lower the translation-unit ABI level; #### Bug Fixes to C++ Support +- Fixed a false type mismatch when a typedef naming an anonymous enumeration + was used through a C++20 named module and its defining header was subsequently + included. (#GH213299) + - Fixed an issue where `__typeof__` incorrectly rejected cv-qualified function types. - Fixed a bug where top-level CV qualifiers (such as ``const``) were dropped from pointers modified by Microsoft pointer attributes (like ``__ptr32`` and ``__ptr64``) and WebAssembly's ``__funcref``. diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index c46d2d780fad7..974cd9a5debf1 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -2667,6 +2667,33 @@ void Sema::MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New, else New->setTypeSourceInfo(OldTD->getTypeSourceInfo()); + // See https://github.com/llvm/llvm-project/issues/213299 for the case. + // + // Ideally, we shall merge the new enum with the old enum when we + // creating the new enum. But the enum is anonymous and the typedef's name + // come after the enum body, it is too late to merge them. This is the + // choice 10 years ago: a523022b5384d7a0901beea7a5f36ee9c09ba339. Actually + // what we're merging here is the typedef decls. + // + // Then https://github.com/llvm/llvm-project/pull/114240 removes the logic + // to remove the new ED. This the direct trigger for the above issue of + // ambiguous look ups. + // + // We choose to fix the problem by setting the type of new enum to the + // type of old enums. This is consistent with the above call to + // setTypeSourceInfo. + // + // FIXME: The check `M && M->isGlobalModule()` is not necessary but we + // hope to limit the impact of this change. We can relax the check when we + // find similar issue later in other cases. + if (Module *M = OldTag->getOwningModule(); M && M->isGlobalModule()) + if (auto *NewEnum = dyn_cast<EnumDecl>(NewTag)) + if (auto *OldEnum = dyn_cast<EnumDecl>(OldTag)) { + QualType OldEnumType = Context.getCanonicalTagType(OldEnum); + for (auto *ECD : NewEnum->enumerators()) + ECD->setType(OldEnumType); + } + // Make the old tag definition visible. makeMergedDefinitionVisible(Hidden); diff --git a/clang/test/Modules/include-after-imports-enums.cppm b/clang/test/Modules/include-after-imports-enums.cppm index 00affd98e299f..260f6d150f46f 100644 --- a/clang/test/Modules/include-after-imports-enums.cppm +++ b/clang/test/Modules/include-after-imports-enums.cppm @@ -9,13 +9,19 @@ // RUN: %clang_cc1 -std=c++20 %t/use.cpp -fprebuilt-module-path=%t -verify -fsyntax-only //--- enum.h +#pragma once + enum E { Value }; +typedef enum { TypedefValue } TypedefEnum; +void useTypedefEnum(TypedefEnum); + //--- M.cppm module; #include "enum.h" export module M; auto e = Value; +export TypedefEnum typedefEnum; //--- use.cpp // expected-no-diagnostics @@ -23,3 +29,5 @@ import M; #include "enum.h" auto e = Value; +static_assert(__is_same(decltype(TypedefValue), TypedefEnum)); +inline void testTypedefEnum() { useTypedefEnum(TypedefValue); } >From 6b6e9edd2c86d77c6d3e20e84ecb398ddfd00695 Mon Sep 17 00:00:00 2001 From: "yedeng.yd" <[email protected]> Date: Fri, 7 Aug 2026 17:43:20 +0800 Subject: [PATCH 2/4] Update --- clang/lib/Sema/SemaDecl.cpp | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 974cd9a5debf1..721cb357f8381 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -2667,25 +2667,14 @@ void Sema::MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New, else New->setTypeSourceInfo(OldTD->getTypeSourceInfo()); - // See https://github.com/llvm/llvm-project/issues/213299 for the case. + // An anonymous enum is recognized as a redeclaration only when its typedef + // name gets merged, at which point two distinct enum types already exist. + // Retype the new enumerators to the old enum type, matching the typedef + // merge above; otherwise the merged typedef and its enumerators disagree + // on the type (GH213299). // - // Ideally, we shall merge the new enum with the old enum when we - // creating the new enum. But the enum is anonymous and the typedef's name - // come after the enum body, it is too late to merge them. This is the - // choice 10 years ago: a523022b5384d7a0901beea7a5f36ee9c09ba339. Actually - // what we're merging here is the typedef decls. - // - // Then https://github.com/llvm/llvm-project/pull/114240 removes the logic - // to remove the new ED. This the direct trigger for the above issue of - // ambiguous look ups. - // - // We choose to fix the problem by setting the type of new enum to the - // type of old enums. This is consistent with the above call to - // setTypeSourceInfo. - // - // FIXME: The check `M && M->isGlobalModule()` is not necessary but we - // hope to limit the impact of this change. We can relax the check when we - // find similar issue later in other cases. + // FIXME: The global module restriction only limits the impact of this + // change; relax it if the issue shows up in other contexts. if (Module *M = OldTag->getOwningModule(); M && M->isGlobalModule()) if (auto *NewEnum = dyn_cast<EnumDecl>(NewTag)) if (auto *OldEnum = dyn_cast<EnumDecl>(OldTag)) { >From 5e6d7f59fc0adaa04a4116109da2dc26208ae68e Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <[email protected]> Date: Mon, 10 Aug 2026 15:52:53 +0800 Subject: [PATCH 3/4] Update --- clang/lib/Sema/SemaDecl.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 721cb357f8381..143f9ce51bc8a 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -2667,21 +2667,26 @@ void Sema::MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New, else New->setTypeSourceInfo(OldTD->getTypeSourceInfo()); - // An anonymous enum is recognized as a redeclaration only when its typedef - // name gets merged, at which point two distinct enum types already exist. - // Retype the new enumerators to the old enum type, matching the typedef - // merge above; otherwise the merged typedef and its enumerators disagree - // on the type (GH213299). + // An anonymous enum is recognized as a redeclaration only when its + // typedef name gets merged, at which point the new enum and its + // enumerators already have a distinct canonical type. Link the enum + // declarations, but also retype the new enumerators because + // setPreviousDecl() does not update QualTypes built before the merge; + // otherwise the merged typedef and its enumerators disagree on the type + // (GH213299). // // FIXME: The global module restriction only limits the impact of this // change; relax it if the issue shows up in other contexts. - if (Module *M = OldTag->getOwningModule(); M && M->isGlobalModule()) - if (auto *NewEnum = dyn_cast<EnumDecl>(NewTag)) + if (Module *M = OldTag->getOwningModule(); M && M->isGlobalModule()) { + if (auto *NewEnum = dyn_cast<EnumDecl>(NewTag)) { if (auto *OldEnum = dyn_cast<EnumDecl>(OldTag)) { + NewEnum->setPreviousDecl(OldEnum); QualType OldEnumType = Context.getCanonicalTagType(OldEnum); for (auto *ECD : NewEnum->enumerators()) ECD->setType(OldEnumType); } + } + } // Make the old tag definition visible. makeMergedDefinitionVisible(Hidden); >From 6a4fe74e8650dd71ce5c121e8895976d5eb1421d Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <[email protected]> Date: Wed, 12 Aug 2026 15:48:32 +0800 Subject: [PATCH 4/4] Update --- clang/lib/Sema/SemaDecl.cpp | 4 +- clang/unittests/Sema/CMakeLists.txt | 1 + clang/unittests/Sema/SemaModuleTest.cpp | 162 ++++++++++++++++++++++++ 3 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 clang/unittests/Sema/SemaModuleTest.cpp diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 143f9ce51bc8a..ff8784c55ed9a 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -2681,9 +2681,9 @@ void Sema::MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New, if (auto *NewEnum = dyn_cast<EnumDecl>(NewTag)) { if (auto *OldEnum = dyn_cast<EnumDecl>(OldTag)) { NewEnum->setPreviousDecl(OldEnum); - QualType OldEnumType = Context.getCanonicalTagType(OldEnum); + QualType EnumType = Context.getCanonicalTagType(OldEnum); for (auto *ECD : NewEnum->enumerators()) - ECD->setType(OldEnumType); + ECD->setType(EnumType); } } } diff --git a/clang/unittests/Sema/CMakeLists.txt b/clang/unittests/Sema/CMakeLists.txt index 188f6135a60ac..b203bc18eeac8 100644 --- a/clang/unittests/Sema/CMakeLists.txt +++ b/clang/unittests/Sema/CMakeLists.txt @@ -8,6 +8,7 @@ add_distinct_clang_unittest(SemaTests HeuristicResolverTest.cpp GslOwnerPointerInference.cpp SemaLookupTest.cpp + SemaModuleTest.cpp SemaNoloadLookupTest.cpp CLANG_LIBS clangAST diff --git a/clang/unittests/Sema/SemaModuleTest.cpp b/clang/unittests/Sema/SemaModuleTest.cpp new file mode 100644 index 0000000000000..6580249350285 --- /dev/null +++ b/clang/unittests/Sema/SemaModuleTest.cpp @@ -0,0 +1,162 @@ +//===- unittests/Sema/SemaModuleTest.cpp ------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/AST/Decl.h" +#include "clang/Driver/CreateInvocationFromArgs.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Frontend/FrontendAction.h" +#include "clang/Frontend/FrontendActions.h" +#include "clang/Tooling/Tooling.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/Path.h" +#include "llvm/Support/raw_ostream.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace clang::tooling; +using namespace llvm; + +namespace { + +class SemaModuleTest : public ::testing::Test { +protected: + SmallString<256> TestDir; + + void SetUp() override { + ASSERT_FALSE(sys::fs::createUniqueDirectory("sema-module-test", TestDir)); + } + + void TearDown() override { sys::fs::remove_directories(TestDir); } + + void addFile(StringRef Path, StringRef Contents) { + ASSERT_FALSE(sys::path::is_absolute(Path)); + + SmallString<256> AbsPath(TestDir); + sys::path::append(AbsPath, Path); + ASSERT_FALSE(sys::fs::create_directories(sys::path::parent_path(AbsPath))); + + std::error_code EC; + raw_fd_ostream OS(AbsPath, EC); + ASSERT_FALSE(EC); + OS << Contents; + } + + void generateModuleInterface(StringRef ModuleName, StringRef Contents) { + std::string FileName = (ModuleName + ".cppm").str(); + addFile(FileName, Contents); + + CreateInvocationOptions CIOpts; + CIOpts.VFS = vfs::createPhysicalFileSystem(); + DiagnosticOptions DiagOpts; + IntrusiveRefCntPtr<DiagnosticsEngine> Diags = + CompilerInstance::createDiagnostics(*CIOpts.VFS, DiagOpts); + CIOpts.Diags = Diags; + + std::string BMIPath = (TestDir + "/" + ModuleName + ".pcm").str(); + std::string PrebuiltModulePath = + "-fprebuilt-module-path=" + TestDir.str().str(); + const char *Args[] = {"clang++", + "-std=c++20", + "--precompile", + PrebuiltModulePath.c_str(), + "-working-directory", + TestDir.c_str(), + "-I", + TestDir.c_str(), + FileName.c_str()}; + std::shared_ptr<CompilerInvocation> Invocation = + createInvocation(Args, CIOpts); + ASSERT_TRUE(Invocation); + + CompilerInstance Instance(std::move(Invocation)); + Instance.setDiagnostics(Diags); + Instance.getFrontendOpts().OutputFile = BMIPath; + Instance.getFrontendOpts().DisableFree = false; + GenerateModuleInterfaceAction Action; + ASSERT_TRUE(Instance.ExecuteAction(Action)); + ASSERT_FALSE(Diags->hasErrorOccurred()); + } +}; + +struct MergedEnumResult { + bool FoundEnumerator = false; +}; + +class MergedEnumConsumer : public ASTConsumer { +public: + explicit MergedEnumConsumer(MergedEnumResult &Result) : Result(Result) {} + + void HandleTranslationUnit(ASTContext &Context) override { + for (Decl *D : Context.getTranslationUnitDecl()->decls()) { + auto *ED = dyn_cast<EnumDecl>(D); + if (!ED || ED->isFromASTFile()) + continue; + + for (EnumConstantDecl *ECD : ED->enumerators()) { + if (ECD->getName() != "TypedefValue") + continue; + + ASSERT_FALSE(Result.FoundEnumerator); + Result.FoundEnumerator = true; + + ASSERT_EQ(ECD->getDeclContext(), ED); + EXPECT_EQ(ECD->getType(), Context.getCanonicalTagType(ED)); + ASSERT_TRUE(ED->getPreviousDecl()); + EXPECT_TRUE(ED->getPreviousDecl()->isFromASTFile()); + EXPECT_EQ(ECD->getType(), + Context.getCanonicalTagType(ED->getPreviousDecl())); + } + } + } + +private: + MergedEnumResult &Result; +}; + +class MergedEnumAction : public ASTFrontendAction { +public: + explicit MergedEnumAction(MergedEnumResult &Result) : Result(Result) {} + + std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &, + StringRef) override { + return std::make_unique<MergedEnumConsumer>(Result); + } + +private: + MergedEnumResult &Result; +}; + +TEST_F(SemaModuleTest, MergedAnonymousEnumHasConsistentEnumeratorType) { + addFile("enum.h", R"cpp( +#ifndef ENUM_H +#define ENUM_H +typedef enum { TypedefValue } TypedefEnum; +#endif +)cpp"); + generateModuleInterface("M", R"cpp( +module; +#include "enum.h" +export module M; +export TypedefEnum typedefEnum; +)cpp"); + + std::string PrebuiltModulePath = + "-fprebuilt-module-path=" + TestDir.str().str(); + MergedEnumResult Result; + EXPECT_TRUE(runToolOnCodeWithArgs( + std::make_unique<MergedEnumAction>(Result), R"cpp( +import M; +#include "enum.h" +)cpp", + {"-std=c++20", PrebuiltModulePath, "-I", TestDir.str().str()}, + "use.cpp")); + EXPECT_TRUE(Result.FoundEnumerator); +} + +} // namespace _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
