Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

structured binding - wrong value category passed to the get function. #381

Closed
dawidpilarski opened this issue Feb 24, 2021 · 1 comment
Closed
Labels
bug Something isn't working

Comments

@dawidpilarski
Copy link

Having a simple code like:

#include <tuple>

int main()
{
    std::tuple<int, int> tup{2,5};
    auto [a, b] = tup;
}

results in following output from cppinsights:

#include <tuple>

int main()
{
  std::tuple<int, int> tup = std::tuple<int, int>{2, 5};
  std::tuple<int, int> __tup6 = std::tuple<int, int>(tup);
  std::tuple_element<0, std::tuple<int, int> >::type & a = std::get<0UL>(__tup6);
  std::tuple_element<0, std::tuple<int> >::type & b = std::get<1UL>(__tup6);
}

Even though it looks fine, it's not accurate, because, when __tup6 is not of lvalue reference type, which is the case here, then
xvalue is passed to the get function. You can have a look at a wording here:

https://eel.is/c++draft/dcl.struct.bind#4.sentence-7

In either case, e is an lvalue if the type of the entity e is an lvalue reference and an xvalue otherwise.

here e is our __tup6.

Also std::get has its implementations for lvalue references, rvalue references and const lvalue references, which is why it works correctly.

So the code produced should be in fact:

#include <tuple>

int main()
{
  std::tuple<int, int> tup = std::tuple<int, int>{2, 5};
  std::tuple<int, int> __tup6 = std::tuple<int, int>(tup);
  std::tuple_element<0, std::tuple<int, int> >::type & a = std::get<0UL>(std::move(__tup6));
  std::tuple_element<0, std::tuple<int> >::type & b = std::get<1UL>(std::move(__tup6));
}

when structured binding is done as auto [...] and stay as it is right now when it's of the form auto& [...].

Also you can have a look at at the code that will not compile, because of the above:

#include <tuple>
struct myStruct{
    int a = 1;
    int b = 2;
};

template<std::size_t idx>
auto& get(myStruct& str){
    if constexpr (idx == 0) return str.a;
    else return str.b;
}

namespace std{
template <>
struct tuple_size<myStruct>{
    static constexpr std::size_t value = 2;
};

template<>
struct tuple_element<0, myStruct>{
    using type = int&;
};

template<>
struct tuple_element<1, myStruct>{
    using type = int&;
};
}

int main()
{
    myStruct var;
    auto [a, b] = var;
}

it fails with error:

candidate function template not viable: expects an l-value for 1st argument.

@andreasfertig
Copy link
Owner

Hello @dawidpilarski,

thank you for reporting this issue and for the reference to the standard. This made me look deeper into structured bindings and correcting a few other parts.

A fix is on its way.

Andreas

@andreasfertig andreasfertig added the bug Something isn't working label Mar 1, 2021
andreasfertig added a commit that referenced this issue Mar 1, 2021
Fixed #381: Show move into std::get for structured bindings.
andreasfertig added a commit that referenced this issue Feb 8, 2022
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.
andreasfertig added a commit that referenced this issue Feb 8, 2022
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants