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 bindings with pair/tuple not producing references #425

Closed
vittorioromeo opened this issue Dec 1, 2021 · 1 comment
Closed
Labels
bug Something isn't working

Comments

@vittorioromeo
Copy link

The following code:

#include <utility>

int main()
{
    std::pair<int, char> p;
  	auto [a, b] = p;
}

Produces this after being run in cppinsights:

#include <utility>

int main()
{
  std::pair<int, char> p = std::pair<int, char>();
  std::pair<int, char> __p6 = std::pair<int, char>(p);
  int a = std::get<0UL>(static_cast<std::pair<int, char> &&>(__p6));
  char b = std::get<1UL>(static_cast<std::pair<int, char> &&>(__p6));
}

I think that is incorrect. It should be:

Produces this after being run in cppinsights:

#include <utility>

int main()
{
  std::pair<int, char> p = std::pair<int, char>();
  std::pair<int, char> __p6 = std::pair<int, char>(p);
  int& a = std::get<0UL>(static_cast<std::pair<int, char> &&>(__p6));
  char& b = std::get<1UL>(static_cast<std::pair<int, char> &&>(__p6));
}

According to cppreference:

For each identifier, a variable whose type is "reference to std::tuple_element<i, E>::type" is introduced: [...]

@andreasfertig andreasfertig added the bug Something isn't working label Feb 8, 2022
@andreasfertig
Copy link
Owner

Hello @vittorioromeo,

thanks for reporting this. I took me some time but I think I figured the correct behavior out. The patch will contain additional information, but I will explain it here as well for documentation purposes and discussions.

I agree that what C++ Insights showed was wrong. However, the resulting code should be

  std::pair<int, char> p = std::pair<int, char>();
  std::pair<int, char> __p6 = std::pair<int, char>(p);
  int&& a = std::get<0UL>(static_cast<std::pair<int, char> &&>(__p6));
  char&& b = std::get<1UL>(static_cast<std::pair<int, char> &&>(__p6));

Both a and b are rvalue reference because get is called with an rvalue of __p6. That is also what the quote from cppreference describes. The text there seems to agree what the standard says in eel.is/c++draft/dcl.struct.bind#4.

Andreas

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
Fixed #425: Show the correct reference type for structured bindings.
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