https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/122265
>From a9e13ad8d2a7a95d431dddcced611bea1e83b99a Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 9 Jan 2025 10:01:31 +0000 Subject: [PATCH 01/11] [clang][DebugInfo] Expand detection of structured bindings to account for std::get free function When we generate the debug-info for a `VarDecl` we try to determine whether it was introduced as part of a structure binding (aka a "holding var"). If it was, we don't mark it as `artificial`. The heuristic to determine a holding var uses `IgnoreUnlessSpelledInSource` to unwrap the `VarDecl` initializer until we hit a `DeclRefExpr` that refers to a `Decomposition`. For "tuple-like decompositions", Clang will generate a call to a `template<size_t I> Foo get(Bar)` function that retrieves the `Ith` element from the tuple-like structure. If that function is a member function, we get an AST that looks as follows: ``` VarDecl implicit used z1 'std::tuple_element<0, B>::type &&' cinit `-ExprWithCleanups <col:10> 'int' xvalue `-MaterializeTemporaryExpr <col:10> 'int' xvalue extended by Var 0x11d110cf8 'z1' 'std::tuple_element<0, B>::type &&' `-CXXMemberCallExpr <col:10> 'int' `-MemberExpr <col:10> '<bound member function type>' .get 0x11d104390 `-ImplicitCastExpr <col:10> 'B' xvalue <NoOp> `-DeclRefExpr <col:10> 'B' lvalue Decomposition 0x11d1100a8 '' 'B' ``` `IgnoreUnlessSpelledInSource` happily unwraps this down to the `DeclRefExpr`. However, when the `get` helper is a free function (which it is for `std::pair` in libc++ for example), then the AST is: ``` VarDecl col:16 implicit used k 'std::tuple_element<0, const std::tuple<int, int>>::type &' cinit `-CallExpr <col:16> 'const typename tuple_element<0UL, tuple<int, int>>::type':'const int' lvalue adl |-ImplicitCastExpr <col:16> 'const typename tuple_element<0UL, tuple<int, int>>::type &(*)(const tuple<int, int> &) noexcept' <FunctionToPointerDecay> | `-DeclRefExpr <col:16> 'const typename tuple_element<0UL, tuple<int, int>>::type &(const tuple<int, int> &) noexcept' lvalue Function 0x1210262d8 'get' 'const typename tuple_element<0UL, tuple<int, int>>::type &(const tuple<int, int> &) noexcept' (FunctionTemplate 0x11d068088 'get') `-DeclRefExpr <col:16> 'const std::tuple<int, int>' lvalue Decomposition 0x121021518 '' 'const std::tuple<int, int> &' ``` `IgnoreUnlessSpelledInSource` doesn't unwrap this `CallExpr`, so we incorrectly mark the binding as `artificial` in debug-info. This patch adjusts our heuristic to account for `CallExpr` nodes. Fixes https://github.com/llvm/llvm-project/issues/122028 --- clang/lib/CodeGen/CGDebugInfo.cpp | 28 ++++++- .../test/DebugInfo/CXX/structured-binding.cpp | 27 +++++++ .../TestStructuredBinding.py | 11 +++ .../API/lang/cpp/structured-binding/main.cpp | 81 +++++++++++++++++-- 4 files changed, 140 insertions(+), 7 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 0385dbdac869b..4b0f2bce457df 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -87,12 +87,38 @@ static bool IsDecomposedVarDecl(VarDecl const *VD) { if (!Init) return false; + Init = Init->IgnoreUnlessSpelledInSource(); + if (!Init) + return false; + + // For tuple-like decompositions, if the `get` function + // is not a member of the decomposed type, but instead a + // free function (e.g., how std::get is specialized for + // std::pair), then the initializer is a `CallExpr` and + // we need to dig into the argument before unwrapping it + // with IgnoreUnlessSpelledInSource. + if (auto const *CE = llvm::dyn_cast<CallExpr>(Init)) { + if (CE->getNumArgs() == 0) + return false; + + // The first argument will be the type we're decomposing. + // Technically the getter could have more than 1 argument + // (e.g., if they're defaulted arguments), but we don't care + // about them because we just need to check whether the + // first argument is a DecompositionDecl. + auto const *Arg = CE->getArg(0); + if (!Arg) + return false; + + Init = Arg; + } + auto const *RefExpr = llvm::dyn_cast_or_null<DeclRefExpr>(Init->IgnoreUnlessSpelledInSource()); if (!RefExpr) return false; - return llvm::dyn_cast_or_null<DecompositionDecl>(RefExpr->getDecl()); + return llvm::isa_and_nonnull<DecompositionDecl>(RefExpr->getDecl()); } /// Returns true if \ref VD is a compiler-generated variable diff --git a/clang/test/DebugInfo/CXX/structured-binding.cpp b/clang/test/DebugInfo/CXX/structured-binding.cpp index 8032ce85c9e25..7d4919ad52bc7 100644 --- a/clang/test/DebugInfo/CXX/structured-binding.cpp +++ b/clang/test/DebugInfo/CXX/structured-binding.cpp @@ -1,5 +1,6 @@ // RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple %itanium_abi_triple %s -o - | FileCheck %s --implicit-check-not="call void @llvm.dbg.declare" +// CHECK: define noundef i32 @_Z1fv // CHECK: #dbg_declare(ptr %{{[a-z]+}}, ![[VAR_0:[0-9]+]], !DIExpression(), // CHECK: #dbg_declare(ptr %{{[0-9]+}}, ![[VAR_1:[0-9]+]], !DIExpression(), // CHECK: #dbg_declare(ptr %{{[0-9]+}}, ![[VAR_2:[0-9]+]], !DIExpression(DW_OP_plus_uconst, 4), @@ -13,6 +14,9 @@ // CHECK: getelementptr inbounds [2 x i32], ptr {{.*}}, i{{64|32}} 0, i{{64|32}} 1, !dbg ![[A2_DEBUG_LOC:[0-9]+]] // CHECK: getelementptr inbounds nuw { i32, i32 }, ptr {{.*}}, i32 0, i32 1, !dbg ![[C2_DEBUG_LOC:[0-9]+]] // CHECK: extractelement <2 x i32> {{.*}}, i32 1, !dbg ![[V2_DEBUG_LOC:[0-9]+]] +// CHECK: #dbg_declare(ptr %k, ![[VAR_7:[0-9]+]], !DIExpression() +// CHECK: #dbg_declare(ptr %v, ![[VAR_8:[0-9]+]], !DIExpression() +// CHECK: #dbg_declare(ptr %w, ![[VAR_9:[0-9]+]], !DIExpression() // CHECK: ![[VAR_0]] = !DILocalVariable(name: "a" // CHECK: ![[VAR_1]] = !DILocalVariable(name: "x1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) // CHECK: ![[VAR_2]] = !DILocalVariable(name: "y1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) @@ -20,6 +24,9 @@ // CHECK: ![[VAR_4]] = !DILocalVariable(name: "y2", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) // CHECK: ![[VAR_5]] = !DILocalVariable(name: "z1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) // CHECK: ![[VAR_6]] = !DILocalVariable(name: "z2", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) +// CHECK: ![[VAR_7]] = !DILocalVariable(name: "k", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) +// CHECK: ![[VAR_8]] = !DILocalVariable(name: "v", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) +// CHECK: ![[VAR_9]] = !DILocalVariable(name: "w", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) struct A { int x; @@ -45,6 +52,24 @@ struct tuple_size<B> { }; template<unsigned, typename T> struct tuple_element { using type = int; }; + +// Decomposition of tuple-like bindings but where the `get` methods +// are declared as free functions. +struct triple { + int k; + int v; + int w; +}; + +template<> +struct tuple_size<triple> { + static constexpr unsigned value = 3; +}; + +template <unsigned I> int get(triple); +template <> int get<0>(triple p) { return p.k; } +template <> int get<1>(triple p) { return p.v; } +template <> int get<2>(triple p) { return p.w; } } // namespace std int f() { @@ -89,4 +114,6 @@ int f() { // CHECK: ![[V2_DEBUG_LOC]] = !DILocation(line: [[@LINE+1]] v2 // ; + auto [k, v, w] = std::triple{3, 4, 5}; + return x1 + y1 + x2 + y2 + z1 + z2 + k + v + w; } diff --git a/lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py b/lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py index 1e026cf8c237c..7bf65c7633a02 100644 --- a/lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py +++ b/lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py @@ -98,3 +98,14 @@ def test(self): self.expect_expr("tx2", result_value="4") self.expect_expr("ty2", result_value="'z'") self.expect_expr("tz2", result_value="10") + + self.expect("frame variable", + substrs=[ + "tx1 =", + "ty1 =", + "tz1 =", + "tx2 =", + "ty2 =", + "tz2 =", + "mp1 =", + "mp2 ="]) diff --git a/lldb/test/API/lang/cpp/structured-binding/main.cpp b/lldb/test/API/lang/cpp/structured-binding/main.cpp index 3fbfb18dbeff0..b649358ebdf66 100644 --- a/lldb/test/API/lang/cpp/structured-binding/main.cpp +++ b/lldb/test/API/lang/cpp/structured-binding/main.cpp @@ -1,13 +1,80 @@ // Structured binding in C++ can bind identifiers to subobjects of an object. // -// There are three cases we need to test: +// There are four cases we need to test: // 1) arrays -// 2) tuples like objects -// 3) non-static data members +// 2) tuple-like objects with `get` member functions +// 3) tuple-like objects with `get` free functions +// 4) non-static data members // // They can also bind by copy, reference or rvalue reference. -#include <tuple> +struct MyPair { + int m1; + int m2; + + // Helpers to enable tuple-like decomposition. + template <unsigned> int get(); + template <> int get<0>() { return m1; } + template <> int get<1>() { return m2; } +}; + +namespace std { +template <typename T1, typename T2, typename T3> struct mock_tuple { + T1 m1; + T2 m2; + T3 m3; +}; + +template <typename T> struct tuple_size; + +template <unsigned, typename T> struct tuple_element; + +// Helpers to enable tuple-like decomposition for MyPair +template <unsigned I> struct tuple_element<I, MyPair> { + using type = int; +}; + +template <> struct tuple_size<MyPair> { + static constexpr unsigned value = 2; +}; + +// Helpers to enable tuple-like decomposition for mock_tuple +template <typename T1, typename T2, typename T3> +struct tuple_element<0, mock_tuple<T1, T2, T3>> { + using type = T1; +}; + +template <typename T1, typename T2, typename T3> +struct tuple_element<1, mock_tuple<T1, T2, T3>> { + using type = T2; +}; + +template <typename T1, typename T2, typename T3> +struct tuple_element<2, mock_tuple<T1, T2, T3>> { + using type = T3; +}; + +template <typename T1, typename T2, typename T3> +struct tuple_size<mock_tuple<T1, T2, T3>> { + static constexpr unsigned value = 3; +}; + +template <unsigned I, typename T1, typename T2, typename T3> +typename tuple_element<I, mock_tuple<T1, T2, T3>>::type +get(mock_tuple<T1, T2, T3> p) { + switch (I) { + case 0: + return p.m1; + case 1: + return p.m2; + case 2: + return p.m3; + default: + __builtin_trap(); + } +} + +} // namespace std struct A { int x; @@ -54,10 +121,12 @@ int main() { char y{'z'}; int z{10}; - std::tuple<float, char, int> tpl(x, y, z); + std::mock_tuple<float, char, int> tpl{.m1 = x, .m2 = y, .m3 = z}; auto [tx1, ty1, tz1] = tpl; auto &[tx2, ty2, tz2] = tpl; + auto [mp1, mp2] = MyPair{.m1 = 1, .m2 = 2}; + return a1.x + b1 + c1 + d1 + e1 + f1 + a2.y + b2 + c2 + d2 + e2 + f2 + a3.x + b3 + c3 + d3 + e3 + f3 + carr_copy1 + carr_copy2 + carr_copy3 + sarr_copy1 + sarr_copy2 + sarr_copy3 + iarr_copy1 + iarr_copy2 + @@ -65,5 +134,5 @@ int main() { sarr_ref2 + sarr_ref3 + iarr_ref1 + iarr_ref2 + iarr_ref3 + carr_rref1 + carr_rref2 + carr_rref3 + sarr_rref1 + sarr_rref2 + sarr_rref3 + iarr_rref1 + iarr_rref2 + iarr_rref3 + tx1 + ty1 + tz1 + - tx2 + ty2 + tz2; // break here + tx2 + ty2 + tz2 + mp1 + mp2; // break here } >From 91dae190f34d172249cacec189fda55a5414eeb2 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 9 Jan 2025 12:37:56 +0000 Subject: [PATCH 02/11] fixup! python format --- .../lang/cpp/structured-binding/TestStructuredBinding.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py b/lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py index 7bf65c7633a02..5f939ecfbef29 100644 --- a/lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py +++ b/lldb/test/API/lang/cpp/structured-binding/TestStructuredBinding.py @@ -99,7 +99,8 @@ def test(self): self.expect_expr("ty2", result_value="'z'") self.expect_expr("tz2", result_value="10") - self.expect("frame variable", + self.expect( + "frame variable", substrs=[ "tx1 =", "ty1 =", @@ -108,4 +109,6 @@ def test(self): "ty2 =", "tz2 =", "mp1 =", - "mp2 ="]) + "mp2 =", + ], + ) >From 66940003ecec5fdde52521a8df923e72386b3c12 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 9 Jan 2025 18:53:43 +0000 Subject: [PATCH 03/11] fixup! relax regex in test --- clang/test/DebugInfo/CXX/structured-binding.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/DebugInfo/CXX/structured-binding.cpp b/clang/test/DebugInfo/CXX/structured-binding.cpp index 7d4919ad52bc7..c297fce2f0ea4 100644 --- a/clang/test/DebugInfo/CXX/structured-binding.cpp +++ b/clang/test/DebugInfo/CXX/structured-binding.cpp @@ -1,6 +1,6 @@ // RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple %itanium_abi_triple %s -o - | FileCheck %s --implicit-check-not="call void @llvm.dbg.declare" -// CHECK: define noundef i32 @_Z1fv +// CHECK: define {{.*}} i32 @_Z1fv // CHECK: #dbg_declare(ptr %{{[a-z]+}}, ![[VAR_0:[0-9]+]], !DIExpression(), // CHECK: #dbg_declare(ptr %{{[0-9]+}}, ![[VAR_1:[0-9]+]], !DIExpression(), // CHECK: #dbg_declare(ptr %{{[0-9]+}}, ![[VAR_2:[0-9]+]], !DIExpression(DW_OP_plus_uconst, 4), >From 6ea0f3f3d67f783e1de6e26ccfb9cae1be12b407 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 13 Jan 2025 11:33:43 +0000 Subject: [PATCH 04/11] fixup! ignore implicit function call in IgnoreUnlessSpelledInSource --- clang/lib/AST/Expr.cpp | 16 +++++++- clang/lib/CodeGen/CGDebugInfo.cpp | 26 ------------ .../test/DebugInfo/CXX/structured-binding.cpp | 40 ++++++++++++++++++- 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index cdff160067fed..6c8e8eecda4fd 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -3159,10 +3159,24 @@ Expr *Expr::IgnoreUnlessSpelledInSource() { } return E; }; + + auto IgnoreImplicitCallSingleStep = [](Expr *E) { + if (auto *C = dyn_cast<CallExpr>(E)) { + auto NumArgs = C->getNumArgs(); + if (NumArgs == 1 || + (NumArgs > 1 && isa<CXXDefaultArgExpr>(C->getArg(1)))) { + Expr *A = C->getArg(0); + if (A->getSourceRange() == E->getSourceRange()) + return A; + } + } + return E; + }; + return IgnoreExprNodes( this, IgnoreImplicitSingleStep, IgnoreImplicitCastsExtraSingleStep, IgnoreParensOnlySingleStep, IgnoreImplicitConstructorSingleStep, - IgnoreImplicitMemberCallSingleStep); + IgnoreImplicitMemberCallSingleStep, IgnoreImplicitCallSingleStep); } bool Expr::isDefaultArgument() const { diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 4b0f2bce457df..4e3e3d25e24ee 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -87,32 +87,6 @@ static bool IsDecomposedVarDecl(VarDecl const *VD) { if (!Init) return false; - Init = Init->IgnoreUnlessSpelledInSource(); - if (!Init) - return false; - - // For tuple-like decompositions, if the `get` function - // is not a member of the decomposed type, but instead a - // free function (e.g., how std::get is specialized for - // std::pair), then the initializer is a `CallExpr` and - // we need to dig into the argument before unwrapping it - // with IgnoreUnlessSpelledInSource. - if (auto const *CE = llvm::dyn_cast<CallExpr>(Init)) { - if (CE->getNumArgs() == 0) - return false; - - // The first argument will be the type we're decomposing. - // Technically the getter could have more than 1 argument - // (e.g., if they're defaulted arguments), but we don't care - // about them because we just need to check whether the - // first argument is a DecompositionDecl. - auto const *Arg = CE->getArg(0); - if (!Arg) - return false; - - Init = Arg; - } - auto const *RefExpr = llvm::dyn_cast_or_null<DeclRefExpr>(Init->IgnoreUnlessSpelledInSource()); if (!RefExpr) diff --git a/clang/test/DebugInfo/CXX/structured-binding.cpp b/clang/test/DebugInfo/CXX/structured-binding.cpp index c297fce2f0ea4..38379f814435e 100644 --- a/clang/test/DebugInfo/CXX/structured-binding.cpp +++ b/clang/test/DebugInfo/CXX/structured-binding.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple %itanium_abi_triple %s -o - | FileCheck %s --implicit-check-not="call void @llvm.dbg.declare" +// RUN: %clang_cc1 -std=c++23 -emit-llvm -debug-info-kind=standalone -triple %itanium_abi_triple %s -o - | FileCheck %s --implicit-check-not="call void @llvm.dbg.declare" // CHECK: define {{.*}} i32 @_Z1fv // CHECK: #dbg_declare(ptr %{{[a-z]+}}, ![[VAR_0:[0-9]+]], !DIExpression(), @@ -17,6 +17,10 @@ // CHECK: #dbg_declare(ptr %k, ![[VAR_7:[0-9]+]], !DIExpression() // CHECK: #dbg_declare(ptr %v, ![[VAR_8:[0-9]+]], !DIExpression() // CHECK: #dbg_declare(ptr %w, ![[VAR_9:[0-9]+]], !DIExpression() +// CHECK: #dbg_declare(ptr %m, ![[VAR_10:[0-9]+]], !DIExpression() +// CHECK: #dbg_declare(ptr %n, ![[VAR_11:[0-9]+]], !DIExpression() +// CHECK: #dbg_declare(ptr %s, ![[VAR_12:[0-9]+]], !DIExpression() +// CHECK: #dbg_declare(ptr %p, ![[VAR_13:[0-9]+]], !DIExpression() // CHECK: ![[VAR_0]] = !DILocalVariable(name: "a" // CHECK: ![[VAR_1]] = !DILocalVariable(name: "x1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) // CHECK: ![[VAR_2]] = !DILocalVariable(name: "y1", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) @@ -27,6 +31,10 @@ // CHECK: ![[VAR_7]] = !DILocalVariable(name: "k", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) // CHECK: ![[VAR_8]] = !DILocalVariable(name: "v", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) // CHECK: ![[VAR_9]] = !DILocalVariable(name: "w", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) +// CHECK: ![[VAR_10]] = !DILocalVariable(name: "m", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) +// CHECK: ![[VAR_11]] = !DILocalVariable(name: "n", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) +// CHECK: ![[VAR_12]] = !DILocalVariable(name: "s", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) +// CHECK: ![[VAR_13]] = !DILocalVariable(name: "p", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: {{[0-9]+}}, type: !{{[0-9]+}}) struct A { int x; @@ -41,6 +49,22 @@ struct B { template<> int get<1>() { return z; } }; +struct C { + int w; + int z; + template<int> int get(this C&& self); + template<> int get<0>(this C&& self) { return self.w; } + template<> int get<1>(this C&& self) { return self.z; } +}; + +struct D { + int w; + int z; + template<int> int get(int unused = 0); + template<> int get<0>(int unused) { return w; } + template<> int get<1>(int unused) { return z; } +}; + // Note: the following declarations are necessary for decomposition of tuple-like // structured bindings namespace std { @@ -51,6 +75,16 @@ struct tuple_size<B> { static constexpr unsigned value = 2; }; +template<> +struct tuple_size<C> { + static constexpr unsigned value = 2; +}; + +template<> +struct tuple_size<D> { + static constexpr unsigned value = 2; +}; + template<unsigned, typename T> struct tuple_element { using type = int; }; // Decomposition of tuple-like bindings but where the `get` methods @@ -115,5 +149,7 @@ int f() { v2 // ; auto [k, v, w] = std::triple{3, 4, 5}; - return x1 + y1 + x2 + y2 + z1 + z2 + k + v + w; + auto [m, n] = C{2, 3}; + auto [s, p] = D{2, 3}; + return x1 + y1 + x2 + y2 + z1 + z2 + k + v + w + m + n + s + p; } >From 1e2ca2364d7d32fa63851e731eed3a27438f929b Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 13 Jan 2025 14:59:19 +0000 Subject: [PATCH 05/11] fixup! revert NFC change --- clang/lib/CodeGen/CGDebugInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 4e3e3d25e24ee..0385dbdac869b 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -92,7 +92,7 @@ static bool IsDecomposedVarDecl(VarDecl const *VD) { if (!RefExpr) return false; - return llvm::isa_and_nonnull<DecompositionDecl>(RefExpr->getDecl()); + return llvm::dyn_cast_or_null<DecompositionDecl>(RefExpr->getDecl()); } /// Returns true if \ref VD is a compiler-generated variable >From b156e5ac429f37a316281b82ba62c5e1363b1af2 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 13 Jan 2025 16:07:49 +0000 Subject: [PATCH 06/11] fixup! add AST unit-test --- clang/unittests/AST/ASTTraverserTest.cpp | 67 ++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/clang/unittests/AST/ASTTraverserTest.cpp b/clang/unittests/AST/ASTTraverserTest.cpp index 988e81d8e51de..6da690a974848 100644 --- a/clang/unittests/AST/ASTTraverserTest.cpp +++ b/clang/unittests/AST/ASTTraverserTest.cpp @@ -1174,6 +1174,12 @@ struct Pair int x, y; }; +// Tuple-like structure with a `get` method that has a default argument. +struct Pair2 +{ + int x, y; +}; + // Note: these utilities are required to force binding to tuple like structure namespace std { @@ -1188,6 +1194,12 @@ namespace std static constexpr size_t value = 2; }; + template <> + struct tuple_size<Pair2> + { + static constexpr size_t value = 2; + }; + template <size_t I, class T> struct tuple_element { @@ -1199,12 +1211,17 @@ namespace std template <size_t I> int &&get(Pair &&p); +template <size_t I> +int &&get(Pair2 &&p, int unused = 0); + void decompTuple() { Pair p{1, 2}; auto [a, b] = p; a = 3; + + auto [c, d] = Pair2{3, 4}; } )cpp", @@ -1586,6 +1603,56 @@ DecompositionDecl '' |-DeclRefExpr 'p' |-BindingDecl 'a' `-BindingDecl 'b' +)cpp"); + } + + { + auto FN = ast_matchers::match( + functionDecl(hasName("decompTuple"), hasDescendant(callExpr(hasAncestor(varDecl(hasName("a"), + hasAncestor(bindingDecl())))).bind("decomp_call"))), + AST2->getASTContext()); + EXPECT_EQ(FN.size(), 1u); + + EXPECT_EQ( + dumpASTString(TK_AsIs, FN[0].getNodeAs<CallExpr>("decomp_call")), + R"cpp( +CallExpr +|-ImplicitCastExpr +| `-DeclRefExpr 'get' +`-ImplicitCastExpr + `-DeclRefExpr '' +)cpp"); + + EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource, + FN[0].getNodeAs<CallExpr>("decomp_call")), + R"cpp( +DeclRefExpr '' +)cpp"); + } + + { + auto FN = ast_matchers::match( + functionDecl(hasName("decompTuple"), hasDescendant(callExpr(hasAncestor(varDecl(hasName("c"), + hasAncestor(bindingDecl())))).bind("decomp_call_with_default"))), + AST2->getASTContext()); + EXPECT_EQ(FN.size(), 1u); + + EXPECT_EQ(dumpASTString(TK_AsIs, FN[0].getNodeAs<CallExpr>( + "decomp_call_with_default")), + R"cpp( +CallExpr +|-ImplicitCastExpr +| `-DeclRefExpr 'get' +|-ImplicitCastExpr +| `-DeclRefExpr '' +`-CXXDefaultArgExpr + `-IntegerLiteral +)cpp"); + + EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource, + FN[0].getNodeAs<CallExpr>("decomp_call_with_default")), + R"cpp( +DeclRefExpr '' )cpp"); } } >From 23b879c5872618eedf8f608453049a7f9140c383 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Mon, 13 Jan 2025 16:16:16 +0000 Subject: [PATCH 07/11] fixup! clang-format --- clang/unittests/AST/ASTTraverserTest.cpp | 26 +++++++++++++++--------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/clang/unittests/AST/ASTTraverserTest.cpp b/clang/unittests/AST/ASTTraverserTest.cpp index 6da690a974848..bcbf01b6b0385 100644 --- a/clang/unittests/AST/ASTTraverserTest.cpp +++ b/clang/unittests/AST/ASTTraverserTest.cpp @@ -1608,14 +1608,16 @@ DecompositionDecl '' { auto FN = ast_matchers::match( - functionDecl(hasName("decompTuple"), hasDescendant(callExpr(hasAncestor(varDecl(hasName("a"), - hasAncestor(bindingDecl())))).bind("decomp_call"))), + functionDecl(hasName("decompTuple"), + hasDescendant(callExpr(hasAncestor(varDecl( + hasName("a"), + hasAncestor(bindingDecl())))) + .bind("decomp_call"))), AST2->getASTContext()); EXPECT_EQ(FN.size(), 1u); - EXPECT_EQ( - dumpASTString(TK_AsIs, FN[0].getNodeAs<CallExpr>("decomp_call")), - R"cpp( + EXPECT_EQ(dumpASTString(TK_AsIs, FN[0].getNodeAs<CallExpr>("decomp_call")), + R"cpp( CallExpr |-ImplicitCastExpr | `-DeclRefExpr 'get' @@ -1632,8 +1634,11 @@ DeclRefExpr '' { auto FN = ast_matchers::match( - functionDecl(hasName("decompTuple"), hasDescendant(callExpr(hasAncestor(varDecl(hasName("c"), - hasAncestor(bindingDecl())))).bind("decomp_call_with_default"))), + functionDecl(hasName("decompTuple"), + hasDescendant(callExpr(hasAncestor(varDecl( + hasName("c"), + hasAncestor(bindingDecl())))) + .bind("decomp_call_with_default"))), AST2->getASTContext()); EXPECT_EQ(FN.size(), 1u); @@ -1649,9 +1654,10 @@ CallExpr `-IntegerLiteral )cpp"); - EXPECT_EQ(dumpASTString(TK_IgnoreUnlessSpelledInSource, - FN[0].getNodeAs<CallExpr>("decomp_call_with_default")), - R"cpp( + EXPECT_EQ( + dumpASTString(TK_IgnoreUnlessSpelledInSource, + FN[0].getNodeAs<CallExpr>("decomp_call_with_default")), + R"cpp( DeclRefExpr '' )cpp"); } >From 549d18c4e3c96c2ab56cfaa895fffa47ac8604e9 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 16 Jan 2025 18:13:48 +0000 Subject: [PATCH 08/11] fixup! add comment --- clang/lib/AST/Expr.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 6c8e8eecda4fd..66dccb8b59922 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -3160,6 +3160,8 @@ Expr *Expr::IgnoreUnlessSpelledInSource() { return E; }; + // Used when Clang generates calls to std::get for decomposing + // structured bindings. auto IgnoreImplicitCallSingleStep = [](Expr *E) { if (auto *C = dyn_cast<CallExpr>(E)) { auto NumArgs = C->getNumArgs(); >From f61bde658b56223541c73be88f552e20200997e6 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Wed, 3 Sep 2025 16:07:28 +0100 Subject: [PATCH 09/11] fixup! fix test mismerge --- clang/test/DebugInfo/CXX/structured-binding.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/clang/test/DebugInfo/CXX/structured-binding.cpp b/clang/test/DebugInfo/CXX/structured-binding.cpp index 38379f814435e..2c7e3dd64adbd 100644 --- a/clang/test/DebugInfo/CXX/structured-binding.cpp +++ b/clang/test/DebugInfo/CXX/structured-binding.cpp @@ -117,6 +117,9 @@ int f() { auto &[c1, c2] = cmplx; int vctr __attribute__ ((vector_size (sizeof(int)*2)))= {1, 2}; auto &[v1, v2] = vctr; + auto [k, v, w] = std::triple{3, 4, 5}; + auto [m, n] = C{2, 3}; + auto [s, p] = D{2, 3}; return // x1 // + // @@ -148,8 +151,4 @@ int f() { // CHECK: ![[V2_DEBUG_LOC]] = !DILocation(line: [[@LINE+1]] v2 // ; - auto [k, v, w] = std::triple{3, 4, 5}; - auto [m, n] = C{2, 3}; - auto [s, p] = D{2, 3}; - return x1 + y1 + x2 + y2 + z1 + z2 + k + v + w + m + n + s + p; } >From 7bc878e61d3d7bbc448cb6658b55bbf70e4299b3 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 4 Sep 2025 11:21:00 +0100 Subject: [PATCH 10/11] fixup! narrow heuristic for ignoring CallExpr --- clang/lib/AST/Expr.cpp | 44 +++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 66dccb8b59922..b74edfda64493 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -2544,6 +2544,19 @@ Stmt *BlockExpr::getBody() { //===----------------------------------------------------------------------===// // Generic Expression Routines //===----------------------------------------------------------------------===// +namespace { +/// Helper to determine wether \c E is a CXXConstructExpr constructing +/// a DecompositionDecl. Used to skip Clang-generated calls to std::get +/// for structured bindings. +bool IsDecompositionDeclRefExpr(const Expr *E) { + const Expr *Unrwapped = E->IgnoreUnlessSpelledInSource(); + const DeclRefExpr *Ref = llvm::dyn_cast_or_null<DeclRefExpr>(Unrwapped); + if (!Ref) + return false; + + return llvm::isa_and_nonnull<DecompositionDecl *>(Ref->getDecl()); +} +} // namespace bool Expr::isReadIfDiscardedInCPlusPlus11() const { // In C++11, discarded-value expressions of a certain form are special, @@ -3163,15 +3176,28 @@ Expr *Expr::IgnoreUnlessSpelledInSource() { // Used when Clang generates calls to std::get for decomposing // structured bindings. auto IgnoreImplicitCallSingleStep = [](Expr *E) { - if (auto *C = dyn_cast<CallExpr>(E)) { - auto NumArgs = C->getNumArgs(); - if (NumArgs == 1 || - (NumArgs > 1 && isa<CXXDefaultArgExpr>(C->getArg(1)))) { - Expr *A = C->getArg(0); - if (A->getSourceRange() == E->getSourceRange()) - return A; - } - } + auto *C = dyn_cast<CallExpr>(E); + if (!C) + return E; + + // Looking for calls to a std::get, which usually just takes + // 1 argument (i.e., the structure being decomposed). If it has + // more than 1 argument, the others need to be defaulted. + unsigned NumArgs = C->getNumArgs(); + if (NumArgs > 1 && !isa<CXXDefaultArgExpr>(C->getArg(1))) + return E; + + Expr *A = C->getArg(0); + + // This was spelled out in source. Don't ignore. + if (A->getSourceRange() != E->getSourceRange()) + return E; + + // If the argument refers to a DecompositionDecl construction, + // ignore it. + if (IsDecompositionDeclRefExpr(A)) + return A; + return E; }; >From 4ad1c79e37862ec98331be67a21c5a7dc8c6af72 Mon Sep 17 00:00:00 2001 From: Michael Buch <michaelbuc...@gmail.com> Date: Thu, 4 Sep 2025 11:34:42 +0100 Subject: [PATCH 11/11] fixup! fix isa call --- clang/lib/AST/Expr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index b74edfda64493..bbd64113e8d77 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -2554,7 +2554,7 @@ bool IsDecompositionDeclRefExpr(const Expr *E) { if (!Ref) return false; - return llvm::isa_and_nonnull<DecompositionDecl *>(Ref->getDecl()); + return llvm::isa_and_nonnull<DecompositionDecl>(Ref->getDecl()); } } // namespace _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits