From f317d9a3a6cfb2abf39dc1c48bb724b46b904a64 Mon Sep 17 00:00:00 2001 From: Andreas Fertig Date: Mon, 7 Feb 2022 13:35:18 +0100 Subject: [PATCH] Fixed #425: Show the correct reference type for structured bindings. This patch changes the behavior of how the resulting reference type of a binding is determined. Now, the code uses the holding variables type information of a `BindingDecl`. This seems to be consistent with what Clang does. The reference kind of a binding is determined by the declaration of the structured binding. In [dcl.struct.bind] the standard says that the reference type for a binding is an lvalue reference if the initializer is an lvalue and otherwise a rvalue reference. The initializer here donates to the hidden object, referenced as e. WE declare this implicitly by declaring the structured binding. We get an rvalue reference in case we declare the structured binding as a non-reference like this: ``` auto [a, b, c] ``` We get an lvalue reference if the structured binding is declared as this: ``` auto& [a, b, c] ``` In the rvalue case we could profit from move because the contents of the implicitly created object can be moved out. If we have a reference to the original object moving wouldn't be a good ting to do, hence an lvalue reference. `std::get` has overloads for both l- and rvalues and returns the respective reference type for each overload. This behavior is observable thanks to #381 which shows the implicit cast of the hidden object to a rvalue. --- CodeGenerator.cpp | 16 ++-- tests/Issue131.expect | 4 +- tests/Issue20.expect | 4 +- tests/Issue381.expect | 4 +- tests/Issue425.cpp | 7 ++ tests/Issue425.expect | 11 +++ tests/StructuredBindingsHandler10Test.cpp | 12 +++ tests/StructuredBindingsHandler10Test.expect | 20 +++++ tests/StructuredBindingsHandler3Test.expect | 18 ++-- tests/StructuredBindingsHandler5Test.expect | 4 +- tests/StructuredBindingsHandler6Test.expect | 10 +-- tests/StructuredBindingsHandler8Test.cpp | 10 +++ tests/StructuredBindingsHandler8Test.expect | 19 ++++ tests/StructuredBindingsHandler9Test.expect | 4 +- tests/TupeInRangeBasedForTest.expect | 4 +- tests/dcl.struct.bind.Test.cpp | 86 ++++++++++++++++++ tests/dcl.struct.bind.Test.expect | 91 ++++++++++++++++++++ 17 files changed, 291 insertions(+), 33 deletions(-) create mode 100644 tests/Issue425.cpp create mode 100644 tests/Issue425.expect create mode 100644 tests/StructuredBindingsHandler10Test.cpp create mode 100644 tests/StructuredBindingsHandler10Test.expect create mode 100644 tests/StructuredBindingsHandler8Test.cpp create mode 100644 tests/StructuredBindingsHandler8Test.expect create mode 100644 tests/dcl.struct.bind.Test.cpp create mode 100644 tests/dcl.struct.bind.Test.expect diff --git a/CodeGenerator.cpp b/CodeGenerator.cpp index e4c1b4fc..cc2224a6 100644 --- a/CodeGenerator.cpp +++ b/CodeGenerator.cpp @@ -3895,7 +3895,6 @@ void CodeGenerator::InsertArg(const BindingDecl*) void StructuredBindingsCodeGenerator::InsertArg(const BindingDecl* stmt) { - // Assume that we are looking at a builtin type. We have to construct the variable declaration information. const auto* bindingStmt = stmt->getBinding(); // In a dependent context we have no binding and with that no type. Leave this as it is, we are looking at a @@ -3907,17 +3906,20 @@ void StructuredBindingsCodeGenerator::InsertArg(const BindingDecl* stmt) // Assume that we are looking at a builtin type. We have to construct the variable declaration information. auto type = stmt->getType(); - // If we have holding var we are looking at a user defined type like tuple and those the defaults from above are + // If we have a holding var we are looking at a user defined type like tuple and those the defaults from above are // wrong. This type contains the variable declaration so we insert this. if(const auto* holdingVar = stmt->getHoldingVar()) { - // A rvalue reference boils down to just the type. If it is a reference then it is a lvalue reference at this - // point. Hence we need to strip the &&. + // Initial paper: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0144r0.pdf + + // The type of the binding depends on the initializer. In case the initializer is an lvalue we get a T&, + // otherwise a T&&. We typically look at an lvalue if the decomposition declaration was auto& [a,b]. Note the & + // here We have a rvalue in case the decomposition declaration was auto [a,b]. Note no reference. The standard + // std::get returns a lvalue reference in case e in get(e) is an lvalue, otherwise it returns an rvalue + // reference because then the call is get(std::move(e)) type = holdingVar->getType().getCanonicalType(); - if(type->isRValueReferenceType()) { - type = type.getNonReferenceType(); - } bindingStmt = holdingVar->getAnyInitializer(); + } else if(not type->isLValueReferenceType()) { type = stmt->getASTContext().getLValueReferenceType(type); } diff --git a/tests/Issue131.expect b/tests/Issue131.expect index 079c1093..274d481f 100644 --- a/tests/Issue131.expect +++ b/tests/Issue131.expect @@ -4,6 +4,6 @@ std::tuple foo(); std::tuple __foo5 = foo(); -int a = std::get<0UL>(static_cast &&>(__foo5)); -float b = std::get<1UL>(static_cast &&>(__foo5)); +int && a = std::get<0UL>(static_cast &&>(__foo5)); +float && b = std::get<1UL>(static_cast &&>(__foo5)); diff --git a/tests/Issue20.expect b/tests/Issue20.expect index 95d51906..4011e201 100644 --- a/tests/Issue20.expect +++ b/tests/Issue20.expect @@ -4,8 +4,8 @@ int main() { std::map map = std::map, std::allocator > >{std::initializer_list >{std::pair{1, 2}}, std::less()}; std::pair __operator6 = std::pair(map.begin().operator*()); - const int key = std::get<0UL>(static_cast &&>(__operator6)); - int value = std::get<1UL>(static_cast &&>(__operator6)); + const int && key = std::get<0UL>(static_cast &&>(__operator6)); + int && value = std::get<1UL>(static_cast &&>(__operator6)); return 0; } diff --git a/tests/Issue381.expect b/tests/Issue381.expect index d3ac4de3..3c2f236e 100644 --- a/tests/Issue381.expect +++ b/tests/Issue381.expect @@ -4,8 +4,8 @@ int main() { std::tuple tup = std::tuple{2, 5}; std::tuple __tup6 = std::tuple(tup); - int a = std::get<0UL>(static_cast &&>(__tup6)); - int b = std::get<1UL>(static_cast &&>(__tup6)); + int && a = std::get<0UL>(static_cast &&>(__tup6)); + int && b = std::get<1UL>(static_cast &&>(__tup6)); return 0; } diff --git a/tests/Issue425.cpp b/tests/Issue425.cpp new file mode 100644 index 00000000..6491f21c --- /dev/null +++ b/tests/Issue425.cpp @@ -0,0 +1,7 @@ +#include + +int main() +{ + std::pair p; + auto [a, b] = p; +} diff --git a/tests/Issue425.expect b/tests/Issue425.expect new file mode 100644 index 00000000..cbd9cfd9 --- /dev/null +++ b/tests/Issue425.expect @@ -0,0 +1,11 @@ +#include + +int main() +{ + std::pair p = std::pair(); + std::pair __p6 = std::pair(p); + int && a = std::get<0UL>(static_cast &&>(__p6)); + char && b = std::get<1UL>(static_cast &&>(__p6)); + return 0; +} + diff --git a/tests/StructuredBindingsHandler10Test.cpp b/tests/StructuredBindingsHandler10Test.cpp new file mode 100644 index 00000000..8cf91bad --- /dev/null +++ b/tests/StructuredBindingsHandler10Test.cpp @@ -0,0 +1,12 @@ +// Source: https://en.cppreference.com/w/cpp/language/structured_binding +#include + +float x{}; +char y{}; +int z{}; + +std::tuple tpl(x,std::move(y),z); +const auto& [a,b,c] = tpl; +// a names a structured binding that refers to x; decltype(a) is float& +// b names a structured binding that refers to y; decltype(b) is char&& +// c names a structured binding that refers to the 3rd element of tpl; decltype(c) is const int diff --git a/tests/StructuredBindingsHandler10Test.expect b/tests/StructuredBindingsHandler10Test.expect new file mode 100644 index 00000000..ea1c07e3 --- /dev/null +++ b/tests/StructuredBindingsHandler10Test.expect @@ -0,0 +1,20 @@ +// Source: https://en.cppreference.com/w/cpp/language/structured_binding +#include + +float x = {}; + +char y = {}; + +int z = {}; + + +std::tuple tpl = std::tuple(x, std::move(y), z); + +const std::tuple & __tpl9 = tpl; +float & a = std::get<0UL>(__tpl9); +char & b = std::get<1UL>(__tpl9); +const int & c = std::get<2UL>(__tpl9); + +// a names a structured binding that refers to x; decltype(a) is float& +// b names a structured binding that refers to y; decltype(b) is char&& +// c names a structured binding that refers to the 3rd element of tpl; decltype(c) is const int diff --git a/tests/StructuredBindingsHandler3Test.expect b/tests/StructuredBindingsHandler3Test.expect index a14cc5e8..b0a83faa 100644 --- a/tests/StructuredBindingsHandler3Test.expect +++ b/tests/StructuredBindingsHandler3Test.expect @@ -105,15 +105,15 @@ namespace constant { Q q = Q(); Q __q17 = Q(q); - int a = get<0UL>(static_cast(__q17)); - int b = get<1UL>(static_cast(__q17)); - int c = get<2UL>(static_cast(__q17)); + int && a = get<0UL>(static_cast(__q17)); + int && b = get<1UL>(static_cast(__q17)); + int && c = get<2UL>(static_cast(__q17)); inline constexpr bool f() { Q __q19 = Q(q); - int a = get<0UL>(static_cast(__q19)); - int b = get<1UL>(static_cast(__q19)); - int c = get<2UL>(static_cast(__q19)); + int && a = get<0UL>(static_cast(__q19)); + int && b = get<1UL>(static_cast(__q19)); + int && c = get<2UL>(static_cast(__q19)); return ((a == 0) && (b == 1)) && (c == 4); } inline constexpr int g() @@ -121,9 +121,9 @@ namespace constant int * p = nullptr; { Q __q26 = Q(q); - int a = get<0UL>(static_cast(__q26)); - int b = get<1UL>(static_cast(__q26)); - int c = get<2UL>(static_cast(__q26)); + int && a = get<0UL>(static_cast(__q26)); + int && b = get<1UL>(static_cast(__q26)); + int && c = get<2UL>(static_cast(__q26)); p = &c; }; return *p; diff --git a/tests/StructuredBindingsHandler5Test.expect b/tests/StructuredBindingsHandler5Test.expect index 74ce091e..0bd738f7 100644 --- a/tests/StructuredBindingsHandler5Test.expect +++ b/tests/StructuredBindingsHandler5Test.expect @@ -152,7 +152,7 @@ int main() Point p = Point{1, 2}; Point & __p64 = p; double & x = __p64.get<0>(); - double y = __p64.get<1>(); + double && y = __p64.get<1>(); printf("x:%lf y:%lf\n", p.GetX(), p.GetY()); ++x; ++y; @@ -161,7 +161,7 @@ int main() constexpr const Point p2 = Point{3, 4}; const Point & __p273 = p2; const double & x2 = __p273.get<0>(); - const double y2 = static_cast(__p273.get<1>()); + const double && y2 = static_cast(__p273.get<1>()); return 0; } diff --git a/tests/StructuredBindingsHandler6Test.expect b/tests/StructuredBindingsHandler6Test.expect index 27c0008c..646d6326 100644 --- a/tests/StructuredBindingsHandler6Test.expect +++ b/tests/StructuredBindingsHandler6Test.expect @@ -115,8 +115,8 @@ int main() { Point p = Point{1, 2}; Point __p58 = Point(p); - double x = static_cast(__p58).get<0>(); - double y = static_cast(__p58).get<1>(); + double && x = static_cast(__p58).get<0>(); + double && y = static_cast(__p58).get<1>(); printf("x:%lf y:%lf\n", p.GetX(), p.GetY()); printf("x:%lf y:%lf\n", x, y); char ar[2] = {7, 8}; @@ -143,9 +143,9 @@ int main() char & c = std::get<1UL>(__muple80); double & d = std::get<2UL>(__muple80); std::tuple __muple82 = std::tuple(muple); - int ii = std::get<0UL>(static_cast &&>(__muple82)); - char cc = std::get<1UL>(static_cast &&>(__muple82)); - double dd = std::get<2UL>(static_cast &&>(__muple82)); + int && ii = std::get<0UL>(static_cast &&>(__muple82)); + char && cc = std::get<1UL>(static_cast &&>(__muple82)); + double && dd = std::get<2UL>(static_cast &&>(__muple82)); return 0; } diff --git a/tests/StructuredBindingsHandler8Test.cpp b/tests/StructuredBindingsHandler8Test.cpp new file mode 100644 index 00000000..f7e0b1ee --- /dev/null +++ b/tests/StructuredBindingsHandler8Test.cpp @@ -0,0 +1,10 @@ +#include + +auto tup = std::tuple{2, 5}; +auto [a1, b1] = tup; + + +int i = 5; +auto tup2 = std::tuple{2, i}; +auto [a2, b2] = tup2; + diff --git a/tests/StructuredBindingsHandler8Test.expect b/tests/StructuredBindingsHandler8Test.expect new file mode 100644 index 00000000..69bd603d --- /dev/null +++ b/tests/StructuredBindingsHandler8Test.expect @@ -0,0 +1,19 @@ +#include + +std::tuple tup = std::tuple{2, 5}; + +std::tuple __tup4 = std::tuple(tup); +int && a1 = std::get<0UL>(static_cast &&>(__tup4)); +int && b1 = std::get<1UL>(static_cast &&>(__tup4)); + + + +int i = 5; + +std::tuple tup2 = std::tuple{2, i}; + +std::tuple __tup29 = std::tuple(tup2); +int && a2 = std::get<0UL>(static_cast &&>(__tup29)); +int & b2 = std::get<1UL>(static_cast &&>(__tup29)); + + diff --git a/tests/StructuredBindingsHandler9Test.expect b/tests/StructuredBindingsHandler9Test.expect index 1b5a1a64..7dd06434 100644 --- a/tests/StructuredBindingsHandler9Test.expect +++ b/tests/StructuredBindingsHandler9Test.expect @@ -4,8 +4,8 @@ std::tuple foo = std::tuple(); std::tuple __foo5 = std::tuple(foo); -int a = std::get<0UL>(static_cast &&>(__foo5)); -float b = std::get<1UL>(static_cast &&>(__foo5)); +int && a = std::get<0UL>(static_cast &&>(__foo5)); +float && b = std::get<1UL>(static_cast &&>(__foo5)); std::tuple & __foo6 = foo; int & ra = std::get<0UL>(__foo6); diff --git a/tests/TupeInRangeBasedForTest.expect b/tests/TupeInRangeBasedForTest.expect index 8bf428c0..3c52360f 100644 --- a/tests/TupeInRangeBasedForTest.expect +++ b/tests/TupeInRangeBasedForTest.expect @@ -28,8 +28,8 @@ int main() std::__wrap_iter, std::allocator >, int> *> __end1 = __range1.end(); for(; std::operator!=(__begin1, __end1); __begin1.operator++()) { std::tuple, std::allocator >, int> __operator15 = std::tuple, std::allocator >, int>(__begin1.operator*()); - std::basic_string, std::allocator > s = std::get<0UL>(static_cast, std::allocator >, int> &&>(__operator15)); - int n = std::get<1UL>(static_cast, std::allocator >, int> &&>(__operator15)); + std::basic_string, std::allocator > && s = std::get<0UL>(static_cast, std::allocator >, int> &&>(__operator15)); + int && n = std::get<1UL>(static_cast, std::allocator >, int> &&>(__operator15)); printf("c=%s, n=%d\n", s.c_str(), n); } diff --git a/tests/dcl.struct.bind.Test.cpp b/tests/dcl.struct.bind.Test.cpp new file mode 100644 index 00000000..2dabe1d4 --- /dev/null +++ b/tests/dcl.struct.bind.Test.cpp @@ -0,0 +1,86 @@ + +// p1 +// cv := denote the cv-qualifiers in the decl-specifier-seq +// S := consist of the storage-class-specifiers of the decl-specifier-seq (if any) +// +// 1) a variable with a unique name e is introduced. +// If the assignment-expression in the initializer has array type A and no ref-qualifier is present, e is defined by: +// +// attribute-specifier-seq_{opt} S cv A e ; +// +// otherwise e is defined as-if by: +// +// attribute-specifier-seq_{opt} decl-specifier-seq ref-qualifier_{opt} e initializer ; +// +// The type of the id-expression e is called E. +// +// Note: E is never a reference type +// +// +// -- tuple -- +// Let i be an index of type std::size_t corresponding to vi +// either: +// e.get() +// get(e) +// +// In either case, +// - e is an lvalue if the type of the entity e is an lvalue reference and +// - an xvalue otherwise. +// +// -> auto& [a ,b ] -> e := lvalue +// -> auto [a ,b ] -> e := xvalue +// +// T_i := std::tuple_element::type +// U_i := either +// - T_i&: if the initializer is an lvalue, +// - T_i&&: an rvalue reference otherwise, +// +// variables are introduced with unique names r_i as follows: +// +// S U_i r_i = initializer ; +// +// Each vi is the name of an lvalue of type Ti that refers to the object bound to ri; the referenced type is Ti. +// + + +// The lvalue is a bit-field if that member is a bit-field. [Example: +// struct S { int x1 : 2; volatile double y1; }; +// S f(); +// const auto [ x, y ] = f(); +// The type of the id-expression x is “const int”, the type of the id-expression y is “const volatile double”. —end example] + + +// For each identifier, a variable whose type is "reference to std::tuple_element::type" is introduced: lvalue reference if its corresponding initializer is an lvalue, rvalue reference otherwise. The initializer for the i-th variable is +// - e.get(), if lookup for the identifier get in the scope of E by class member access lookup finds at least one declaration that is a function template whose first template parameter is a non-type parameter +// - Otherwise, get(e), where get is looked up by argument-dependent lookup only, ignoring non-ADL lookup. +// +// The initializer for the new variable is e.get or get(e). +// Here the overload of get that is called is a rvalue in case we use auto and an lvalue in case we use auto& + + + +#include +#include + +// https://en.cppreference.com/w/cpp/language/structured_binding +float x{}; +char y{}; +int z{}; + +std::tuple tpl(x,std::move(y),z); +//auto tpl = std::tuple{x,std::move(y),z}; +auto& [a,b,c] = tpl; +// a names a structured binding that refers to x; decltype(a) is float& +// b names a structured binding that refers to y; decltype(b) is char&& +// c names a structured binding that refers to the 3rd element of tpl; decltype(c) is int + +int main() { + a = 4.5; + c = 5; + +// assert(4.5 == x); +// assert(0 == z); + +// std::cout << a << '\n'; +// std::cout << z << '\n'; +} diff --git a/tests/dcl.struct.bind.Test.expect b/tests/dcl.struct.bind.Test.expect new file mode 100644 index 00000000..f557dd18 --- /dev/null +++ b/tests/dcl.struct.bind.Test.expect @@ -0,0 +1,91 @@ + +// p1 +// cv := denote the cv-qualifiers in the decl-specifier-seq +// S := consist of the storage-class-specifiers of the decl-specifier-seq (if any) +// +// 1) a variable with a unique name e is introduced. +// If the assignment-expression in the initializer has array type A and no ref-qualifier is present, e is defined by: +// +// attribute-specifier-seq_{opt} S cv A e ; +// +// otherwise e is defined as-if by: +// +// attribute-specifier-seq_{opt} decl-specifier-seq ref-qualifier_{opt} e initializer ; +// +// The type of the id-expression e is called E. +// +// Note: E is never a reference type +// +// +// -- tuple -- +// Let i be an index of type std::size_t corresponding to vi +// either: +// e.get() +// get(e) +// +// In either case, +// - e is an lvalue if the type of the entity e is an lvalue reference and +// - an xvalue otherwise. +// +// -> auto& [a ,b ] -> e := lvalue +// -> auto [a ,b ] -> e := xvalue +// +// T_i := std::tuple_element::type +// U_i := either +// - T_i&: if the initializer is an lvalue, +// - T_i&&: an rvalue reference otherwise, +// +// variables are introduced with unique names r_i as follows: +// +// S U_i r_i = initializer ; +// +// Each vi is the name of an lvalue of type Ti that refers to the object bound to ri; the referenced type is Ti. +// + + +// The lvalue is a bit-field if that member is a bit-field. [Example: +// struct S { int x1 : 2; volatile double y1; }; +// S f(); +// const auto [ x, y ] = f(); +// The type of the id-expression x is “const int”, the type of the id-expression y is “const volatile double”. —end example] + + +// For each identifier, a variable whose type is "reference to std::tuple_element::type" is introduced: lvalue reference if its corresponding initializer is an lvalue, rvalue reference otherwise. The initializer for the i-th variable is +// - e.get(), if lookup for the identifier get in the scope of E by class member access lookup finds at least one declaration that is a function template whose first template parameter is a non-type parameter +// - Otherwise, get(e), where get is looked up by argument-dependent lookup only, ignoring non-ADL lookup. +// +// The initializer for the new variable is e.get or get(e). +// Here the overload of get that is called is a rvalue in case we use auto and an lvalue in case we use auto& + + + +#include +#include + +// https://en.cppreference.com/w/cpp/language/structured_binding +float x = {}; + +char y = {}; + +int z = {}; + + +std::tuple tpl = std::tuple(x, std::move(y), z); + +//auto tpl = std::tuple{x,std::move(y),z}; +std::tuple & __tpl72 = tpl; +float & a = std::get<0UL>(__tpl72); +char & b = std::get<1UL>(__tpl72); +int & c = std::get<2UL>(__tpl72); + +// a names a structured binding that refers to x; decltype(a) is float& +// b names a structured binding that refers to y; decltype(b) is char&& +// c names a structured binding that refers to the 3rd element of tpl; decltype(c) is int + +int main() +{ + a = static_cast(4.5); + c = 5; + return 0; +} +