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

CAPTURE of structured binding element no longer works #279

Closed
u3shit opened this issue Aug 24, 2019 · 5 comments
Closed

CAPTURE of structured binding element no longer works #279

u3shit opened this issue Aug 24, 2019 · 5 comments

Comments

@u3shit
Copy link

u3shit commented Aug 24, 2019

Description

Take a look at the example below where I capture a variable from a structured binding. With 2.3.3 and before it worked fine, with 2.3.4 I receive an error like this:

foo.cpp:8:11: error: reference to local binding 'a' declared in enclosing function '_DOCTEST_ANON_FUNC_6'
  CAPTURE(a);
          ^
./doctest/doctest.h:2480:17: note: expanded from macro 'CAPTURE'
#define CAPTURE DOCTEST_CAPTURE
                ^
foo.cpp:7:9: note: 'a' declared here
  auto [a,b] = x;
        ^
1 error generated.

It looks like it was broken in 5e493d3

Steps to reproduce

#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include <doctest/doctest.h>

TEST_CASE("x")
{
  int x[2] = {1,2};
  auto [a,b] = x;
  CAPTURE(a);
}

Compile it with clang++ -std=c++17. It also fails if I use an std::pair/tuple instead of an array.

Extra information

  • doctest version: v2.3.4
  • Operating System: Gentoo Linux amd64
  • Compiler+version: clang 8.0.1 (both with libc++ and gcc's libstdc++)
u3shit added a commit to u3shit/libshit that referenced this issue Aug 24, 2019
@onqtam
Copy link
Member

onqtam commented Aug 25, 2019

INFO() relies on a [&] lambda capture underneath and the problem you've hit is this: https://stackoverflow.com/questions/46114214

........ This works with GCC but fails with clang (haven't tested MSVC).

You could work it around by making a separate auto& name = a; for now and CAPTURE(name); and hopefully the C++ 20/23 standard figures out what to do with structured bindings and lambda captures. Thanks for reporting!

We won't revert to the old implementation of INFO() because it had a horrible limitation - it worked only with lvalues and was a bunch of code which had to be maintained - the new lambda approach is far superior (thanks to @DaanDeMeyer ).

Btw it's impressive that you've tracked down the commit which caused this - in this case it didn't matter much but I wish more bug reports were as thorough as yours!

@u3shit
Copy link
Author

u3shit commented Aug 25, 2019

The funny thing is, I already had a macro for that workaround you suggested because of the rvalue limitation... looks like I no longer need it for rvalues, but now I need structured bindings. Am I the only one who feels c++ standards got buggier recently?
I haven't looked at the implementation, but wouldn't the initializer capture workaround work here? Like [a=&a]() { return *a; }

@onqtam
Copy link
Member

onqtam commented Aug 25, 2019

it won't work in the following case: INFO("aaa" << var1 << var2);

I don't know what the user will be capturing so the capture has to be [&]... I can't generate a proper capture at compile time with macros from whatever the user has passed. Hopefully the bug/hole in the standard will get fixed nicely in the future. Btw I really dislike C++ since I explored Nim and will even be giving a talk how it should replace it at code::dive 2019 :)
https://codedive.pl/index/speaker/name/viktor-kirilov

@onqtam
Copy link
Member

onqtam commented Sep 22, 2019

releasing 2.3.5

@gitsbi
Copy link

gitsbi commented Apr 6, 2022

This is not a bug in a compiler (nor in the standard), but a deliberate decision. The standard says structured bindings are no variables, and thus cannot be captured. (See the discussion here with relevant links.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants