Jan =?utf-8?q?Kokemüller?= <[email protected]> Message-ID: <llvm.org/llvm/llvm-project/pull/[email protected]> In-Reply-To:
https://github.com/llvmbot created https://github.com/llvm/llvm-project/pull/184288 Backport 772b15b3be153b1d2df910057af17926ea227243 765c4e6e8fb25ca999bc19654b5f324df62879ad Requested by: @ChuanqiXu9 >From 47db605b77e31b3916bf8da01514b227a659d0fc Mon Sep 17 00:00:00 2001 From: Chuanqi Xu <[email protected]> Date: Mon, 26 Jan 2026 14:11:25 +0800 Subject: [PATCH 1/2] [C++20] [Modules] Set ManglingContextDecl when we need to mangle a lambda but it's nullptr (#177899) Close https://github.com/llvm/llvm-project/issues/177385 The root cause of the problem is, when we decide to mangle a lamdba in a module interface while the ManglingContextDecl is nullptr, we didn't update ManglingContextDecl. So that the following use of ManglingContextDecl is an invalid value. (cherry picked from commit 772b15b3be153b1d2df910057af17926ea227243) --- clang/lib/Sema/SemaLambda.cpp | 23 ++--- clang/test/Modules/pr177385.cppm | 165 +++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 11 deletions(-) create mode 100644 clang/test/Modules/pr177385.cppm diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp index e1b1cd3e04946..e74fe02bd0cf5 100644 --- a/clang/lib/Sema/SemaLambda.cpp +++ b/clang/lib/Sema/SemaLambda.cpp @@ -298,18 +298,19 @@ Sema::getCurrentMangleNumberContext(const DeclContext *DC) { // definition, as well as the initializers of data members, receive special // treatment. Identify them. Kind = [&]() { + // See discussion in https://github.com/itanium-cxx-abi/cxx-abi/issues/186 + // + // zygoloid: + // Yeah, I think the only cases left where lambdas don't need a + // mangling are when they have (effectively) internal linkage or appear + // in a non-inline function in a non-module translation unit. if (auto *ND = dyn_cast<NamedDecl>(ManglingContextDecl ? ManglingContextDecl - : cast<Decl>(DC))) { - // See discussion in https://github.com/itanium-cxx-abi/cxx-abi/issues/186 - // - // zygoloid: - // Yeah, I think the only cases left where lambdas don't need a - // mangling are when they have (effectively) internal linkage or appear - // in a non-inline function in a non-module translation unit. - Module *M = ND->getOwningModule(); - if (M && M->getTopLevelModule()->isNamedModuleUnit() && - ND->isExternallyVisible()) - return NonInlineInModulePurview; + : cast<Decl>(DC)); + ND && (ND->isInNamedModule() || ND->isFromGlobalModule()) && + ND->isExternallyVisible()) { + if (!ManglingContextDecl) + ManglingContextDecl = const_cast<Decl *>(cast<Decl>(DC)); + return NonInlineInModulePurview; } if (!ManglingContextDecl) diff --git a/clang/test/Modules/pr177385.cppm b/clang/test/Modules/pr177385.cppm new file mode 100644 index 0000000000000..02852ce3b6fe4 --- /dev/null +++ b/clang/test/Modules/pr177385.cppm @@ -0,0 +1,165 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: split-file %s %t +// RUN: mkdir %t/tmp + +// RUN: %clang_cc1 -std=c++26 %t/std.cppm -emit-module-interface -o %t/std.pcm +// RUN: %clang_cc1 -std=c++26 %t/optional.cppm -emit-module-interface -o %t/optional.pcm \ +// RUN: -fmodule-file=std=%t/std.pcm +// RUN: %clang_cc1 -std=c++26 %t/date.format.cppm -emit-module-interface -o %t/date.format.pcm \ +// RUN: -fmodule-file=std=%t/std.pcm \ +// RUN: -fmodule-file=bopt.optional=%t/optional.pcm +// RUN: %clang_cc1 -std=c++26 %t/annotated.cppm -emit-module-interface -o %t/annotated.pcm \ +// RUN: -fmodule-file=std=%t/std.pcm \ +// RUN: -fmodule-file=bopt.optional=%t/optional.pcm \ +// RUN: -fmodule-file=ldgr:date.format=%t/date.format.pcm +// RUN: %clang_cc1 -std=c++26 %t/annotated.format.cppm -emit-module-interface -o %t/annotated.format.pcm \ +// RUN: -fmodule-file=std=%t/std.pcm \ +// RUN: -fmodule-file=bopt.optional=%t/optional.pcm \ +// RUN: -fmodule-file=ldgr:date.format=%t/date.format.pcm \ +// RUN: -fmodule-file=ldgr:annotated=%t/annotated.pcm +// RUN: %clang_cc1 -std=c++26 %t/date.test.cppm -emit-module-interface -o %t/date.test.pcm \ +// RUN: -fmodule-file=std=%t/std.pcm \ +// RUN: -fmodule-file=bopt.optional=%t/optional.pcm \ +// RUN: -fmodule-file=ldgr:date.format=%t/date.format.pcm \ +// RUN: -fmodule-file=ldgr:annotated=%t/annotated.pcm \ +// RUN: -fmodule-file=ldgr:annotated.format=%t/annotated.format.pcm +// RUN: %clang_cc1 -std=c++26 -w %t/annotated.test.cpp -fsyntax-only -verify \ +// RUN: -fmodule-file=std=%t/std.pcm \ +// RUN: -fmodule-file=bopt.optional=%t/optional.pcm \ +// RUN: -fmodule-file=ldgr:date.format=%t/date.format.pcm \ +// RUN: -fmodule-file=ldgr:annotated=%t/annotated.pcm \ +// RUN: -fmodule-file=ldgr:annotated.format=%t/annotated.format.pcm \ +// RUN: -fmodule-file=ldgr:date.testlib=%t/date.test.pcm + +//--- std.cppm +module; +namespace std { +inline namespace __1 { +template <class _Tp> _Tp forward(_Tp); +template <class... _Args> +void __invoke(_Args... __args) noexcept( + noexcept(__builtin_invoke(forward(__args)...))); +using string = char; +struct in_place_t { +} in_place; +template <typename...> struct __traits; +struct Trans_NS___visitation___base { + template <class _Visitor, class... _Vs> + static void __visit_alt(_Visitor, _Vs... __vs) { + __make_fmatrix<_Visitor, decltype(__vs)...>; + } + struct __dispatcher { + template <class _Fp, class... _Vs> static void __dispatch(_Vs... __vs) { + _Fp __f; + __invoke(__f, __vs...); + } + }; + template <class _Fp, class... _Vs> static void __make_dispatch() { + __dispatcher::__dispatch<_Fp, _Vs...>; + } + template <class _Fp, class... _Vs> static void __make_fmatrix_impl() { + __make_dispatch<_Fp, _Vs...>; + } + template <class _Fp, class... _Vs> static void __make_fmatrix() { + __make_fmatrix_impl<_Fp, _Vs...>; + } +}; +template <class> class __dtor; +template <class... _Types> struct __dtor<__traits<_Types...>> { + ~__dtor() { + Trans_NS___visitation___base::__visit_alt([](auto) {}, this); + } +}; +struct __move_constructor : __dtor<__traits<>> {}; +struct __assignment : __move_constructor {}; +struct __impl : __assignment {}; +template <class> struct variant { + __impl __impl_; +}; +} +} +export module std; +export namespace std { +using std::forward; +using std::in_place; +using std::in_place_t; +using std::string; +using std::variant; +} + +//--- optional.cppm +export module bopt.optional; +import std; +namespace bopt { +template <class T> struct wrapper { + template <class Func> wrapper(int, Func f) : value(std::forward(f)()) {} + T value; +}; +template <class T> struct optional_base { + struct union_t { + T value_; + }; + struct repr { + template <class OtherValue> + repr(OtherValue) : un_{0, [] { return union_t{}; }} {} + wrapper<union_t> un_; + }; + template <class... Args> + optional_base(std::in_place_t, Args... args) : repr_{args...} {} + repr repr_; +}; +export template <class T> struct optional : optional_base<T> { + optional(); + template <class U> optional(U) : optional_base<T>{std::in_place, 0} {} + template <class U> constexpr void operator=(U) { + [] {}; + } +}; +} + +//--- date.format.cppm +module ldgr:date.format; +import bopt.optional; +namespace ldgr { +struct date_format { + template <class S> static int parse(S s) { bopt::optional<int> year{s}; } +}; +} + +//--- annotated.cppm +module ldgr:annotated; +import std; +import bopt.optional; +struct lot_annotation { + using valuation_expr_t = std::variant<std::string>; + bopt::optional<valuation_expr_t> valuation_expr_; +}; +struct annotated_amount { + lot_annotation lot_annotation_; +}; + +//--- annotated.format.cppm +module ldgr:annotated.format; +import :date.format; +import :annotated; +namespace ldgr { +struct annotated_format { + static annotated_amount parse_default() { + bopt::optional<int> date; + date = date_format::parse(0); + } +}; +} + +//--- date.test.cppm +module ldgr:date.testlib; +import :date.format; +void date_from_string() { ldgr::date_format::parse(0); } + +//--- annotated.test.cpp +// expected-no-diagnostics +module ldgr:annotatedtest; +import :annotated.format; +import :date.testlib; +int main() { ldgr::annotated_format::parse_default(); } >From a8fee51badd7eec38f6d9ecfc2d5a57c42f75335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Kokem=C3=BCller?= <[email protected]> Date: Thu, 26 Feb 2026 03:27:52 +0100 Subject: [PATCH 2/2] [clang] Don't use `VarDecl` of local variables as `ManglingContextDecl` for lambdas (#179035) Currently, in a C++20 modules context, a `VarDecl` of a local variable can wrongly end up as a `ManglingContextDecl` for a lambda. Fix this by removing `ContextKind::NonInlineInModulePurview` in `Sema::getCurrentMangleNumberContext` and add `IsExternallyVisibleInModulePurview` checks in the appropriate places: - For externally visible functions defined in a module purview, add a check to `isInInlineFunction`, renaming it to `IsInFunctionThatRequiresMangling` - For externally visible variables defined in a module purview, add a new `ContextKind::ExternallyVisibleVariableInModulePurview` and an appropriate check to the `VarDecl` case Fixes #178893 --------- Co-authored-by: Corentin Jabot <[email protected]> Co-authored-by: Chuanqi Xu <[email protected]> (cherry picked from commit 765c4e6e8fb25ca999bc19654b5f324df62879ad) --- clang/lib/Sema/SemaLambda.cpp | 79 +++++++++++++++++++------------- clang/test/Modules/pr178893.cppm | 29 ++++++++++++ 2 files changed, 75 insertions(+), 33 deletions(-) create mode 100644 clang/test/Modules/pr178893.cppm diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp index e74fe02bd0cf5..be7f73dbc267f 100644 --- a/clang/lib/Sema/SemaLambda.cpp +++ b/clang/lib/Sema/SemaLambda.cpp @@ -261,20 +261,6 @@ Sema::createLambdaClosureType(SourceRange IntroducerRange, TypeSourceInfo *Info, return Class; } -/// Determine whether the given context is or is enclosed in an inline -/// function. -static bool isInInlineFunction(const DeclContext *DC) { - while (!DC->isFileContext()) { - if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(DC)) - if (FD->isInlined()) - return true; - - DC = DC->getLexicalParent(); - } - - return false; -} - std::tuple<MangleNumberingContext *, Decl *> Sema::getCurrentMangleNumberContext(const DeclContext *DC) { // Compute the context for allocating mangling numbers in the current @@ -287,32 +273,33 @@ Sema::getCurrentMangleNumberContext(const DeclContext *DC) { DataMember, InlineVariable, TemplatedVariable, + ExternallyVisibleVariableInModulePurview, Concept, - NonInlineInModulePurview } Kind = Normal; bool IsInNonspecializedTemplate = inTemplateInstantiation() || CurContext->isDependentContext(); + // Checks if a VarDecl or FunctionDecl is from a module purview and externally + // visible. These Decls should be treated as "inline" for the purpose of + // mangling in the code below. + // + // See discussion in https://github.com/itanium-cxx-abi/cxx-abi/issues/186 + // + // zygoloid: + // Yeah, I think the only cases left where lambdas don't need a + // mangling are when they have (effectively) internal linkage or + // appear in a non-inline function in a non-module translation unit. + static constexpr auto IsExternallyVisibleInModulePurview = + [](const NamedDecl *ND) -> bool { + return (ND->isInNamedModule() || ND->isFromGlobalModule()) && + ND->isExternallyVisible(); + }; + // Default arguments of member function parameters that appear in a class // definition, as well as the initializers of data members, receive special // treatment. Identify them. Kind = [&]() { - // See discussion in https://github.com/itanium-cxx-abi/cxx-abi/issues/186 - // - // zygoloid: - // Yeah, I think the only cases left where lambdas don't need a - // mangling are when they have (effectively) internal linkage or appear - // in a non-inline function in a non-module translation unit. - if (auto *ND = dyn_cast<NamedDecl>(ManglingContextDecl ? ManglingContextDecl - : cast<Decl>(DC)); - ND && (ND->isInNamedModule() || ND->isFromGlobalModule()) && - ND->isExternallyVisible()) { - if (!ManglingContextDecl) - ManglingContextDecl = const_cast<Decl *>(cast<Decl>(DC)); - return NonInlineInModulePurview; - } - if (!ManglingContextDecl) return Normal; @@ -325,6 +312,9 @@ Sema::getCurrentMangleNumberContext(const DeclContext *DC) { if (Var->getMostRecentDecl()->isInline()) return InlineVariable; + if (IsExternallyVisibleInModulePurview(Var)) + return ExternallyVisibleVariableInModulePurview; + if (Var->getDeclContext()->isRecord() && IsInNonspecializedTemplate) return TemplatedVariable; @@ -344,15 +334,35 @@ Sema::getCurrentMangleNumberContext(const DeclContext *DC) { return Normal; }(); - // Itanium ABI [5.1.7]: + // Determine whether the given context is or is enclosed in a function that + // requires Decl's inside to be mangled, so either: + // - an inline function + // - or a function in a module purview that is externally visible + static constexpr auto IsInFunctionThatRequiresMangling = + [](const DeclContext *DC) -> bool { + while (!DC->isFileContext()) { + if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(DC)) + if (FD->isInlined() || IsExternallyVisibleInModulePurview(FD)) + return true; + + DC = DC->getLexicalParent(); + } + + return false; + }; + + // Itanium ABI [5.1.8]: // In the following contexts [...] the one-definition rule requires closure // types in different translation units to "correspond": switch (Kind) { case Normal: { // -- the bodies of inline or templated functions + // -- the bodies of externally visible functions in a module purview + // (note: this is not yet part of the Itanium ABI, see the linked Github + // discussion above) if ((IsInNonspecializedTemplate && !(ManglingContextDecl && isa<ParmVarDecl>(ManglingContextDecl))) || - isInInlineFunction(CurContext)) { + IsInFunctionThatRequiresMangling(CurContext)) { while (auto *CD = dyn_cast<CapturedDecl>(DC)) DC = CD->getParent(); return std::make_tuple(&Context.getManglingNumberContext(DC), nullptr); @@ -361,7 +371,6 @@ Sema::getCurrentMangleNumberContext(const DeclContext *DC) { return std::make_tuple(nullptr, nullptr); } - case NonInlineInModulePurview: case Concept: // Concept definitions aren't code generated and thus aren't mangled, // however the ManglingContextDecl is important for the purposes of @@ -372,8 +381,12 @@ Sema::getCurrentMangleNumberContext(const DeclContext *DC) { case DefaultArgument: // -- default arguments appearing in class definitions case InlineVariable: + case ExternallyVisibleVariableInModulePurview: case TemplatedVariable: // -- the initializers of inline or templated variables + // -- the initializers of externally visible variables in a module purview + // (note: this is not yet part of the Itanium ABI, see the linked Github + // discussion above) return std::make_tuple( &Context.getManglingNumberContext(ASTContext::NeedExtraManglingDecl, ManglingContextDecl), diff --git a/clang/test/Modules/pr178893.cppm b/clang/test/Modules/pr178893.cppm new file mode 100644 index 0000000000000..e58e183d82aba --- /dev/null +++ b/clang/test/Modules/pr178893.cppm @@ -0,0 +1,29 @@ +// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple -xc++ -emit-llvm -o - %s -w | FileCheck %s + +// CHECK-LABEL: define {{.*}}@_ZZN8PR178893W3mod6format5parseEPiENKUlvE_clEv +// CHECK-LABEL: define {{.*}}@_ZZN8PR178893W3mod6format5parseEPiENKUlvE0_clEv + +export module mod; + +namespace PR178893 { + struct format { + static inline int parse(int* i) + { + int number; + number = [&]() -> int { return i[0]; }(); + + volatile bool b = true; + if (b) { + auto identifier = [&]() -> int { return i[1]; }(); + return identifier; + } + + return number; + } + }; + + int test_format() { + int n[2] = {1, 0}; + return format::parse(n); + } +} _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
