[Bug c++/80737] variant as class member resulting to compile errors

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80737

Jonathan Wakely  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #9 from Jonathan Wakely  ---
Fixed for 7.2

[Bug c++/80737] variant as class member resulting to compile errors

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80737

--- Comment #8 from Jonathan Wakely  ---
Author: redi
Date: Tue Jul 25 18:04:27 2017
New Revision: 250532

URL: https://gcc.gnu.org/viewcvs?rev=250532=gcc=rev
Log:
PR libstdc++/80737 backport std::variant fix from mainline

Backport from mainline
2017-05-20  Tim Shen  

PR libstdc++/80737
* include/std/variant(variant::variant): SFINAE on is_same first.
* testsuite/20_util/variant/any.cc: test case.

Added:
branches/gcc-7-branch/libstdc++-v3/testsuite/20_util/variant/any.cc
Modified:
branches/gcc-7-branch/libstdc++-v3/ChangeLog
branches/gcc-7-branch/libstdc++-v3/include/std/variant

[Bug c++/80737] variant as class member resulting to compile errors

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80737

Jonathan Wakely  changed:

   What|Removed |Added

   Keywords||rejects-valid
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-06-01
   Target Milestone|--- |7.2
 Ever confirmed|0   |1

[Bug c++/80737] variant as class member resulting to compile errors

[timshen at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80737

--- Comment #7 from Tim Shen  ---
Author: timshen
Date: Sun May 28 21:27:30 2017
New Revision: 248548

URL: https://gcc.gnu.org/viewcvs?rev=248548=gcc=rev
Log:
  PR libstdc++/80737
  * include/std/variant(variant::variant): SFINAE on is_same first.
  * testsuite/20_util/variant/any.cc: test case.

Added:
trunk/libstdc++-v3/testsuite/20_util/variant/any.cc
Modified:
trunk/libstdc++-v3/ChangeLog
trunk/libstdc++-v3/include/std/variant

[Bug c++/80737] variant as class member resulting to compile errors

[timshen at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80737

--- Comment #6 from Tim Shen  ---
(In reply to TC from comment #5)
> (In reply to Tim Shen from comment #3)
> > (In reply to TC from comment #1)
> > > Looks like the constraint on the convert-everything constructor needs to
> > > check for is_same, variant> first and short circuit if that's
> > > true.
> > 
> > I'm not quite sure whether we need that short circuit - it's in a SFINAE
> > environment, so if anything goes wrong, there shouldn't be a hard error.
> > Therefore even if I agree that we should check is_same,
> > variant> first, that shouldn't affect the correctness.
> 
> I'm not sure I agree. As http://stackoverflow.com/a/43963065/2756719 points
> out, when you attempt to copy a variant, as part of overload resolution
> the code attempts to check if you can convert a variant to any; that in
> turn queries whether variant is copy constructible,

On this part I agree, that is: if there isn't a short circuiting, then a
instantiation circle will happen: variant(_Tp&&), then any(const
variant&), then is_nothrow_move_constructible, then
variant(_Tp&&).

> and we are very close to turtling all the way down.

The question is: how does the compiler handle an instantiation circle, and can
we rely on that handling?

Clang sees the instantiation circle and bailed out nicely, bascically rewinds
the instantiation stack and SFINAEed out variant(_Tp&&). However, GCC issued a
hard error.

I asked Richard Smith about what's the intended behavior here, and the answer
is that the standard doesn't talk about this.

> 
> It does seem like there might be a frontend problem here, though.

To conclude, there is a front-end implementation mismatch that cause this code
not to compile on GCC. For consistency with other places in libstdc++, I'll
short-circuit the SFINAE expression.

In addition, https://godbolt.org/g/AxUv16 reveals a weird GCC behavior
(with/without -DBUG) that might be a separate bug.

[Bug c++/80737] variant as class member resulting to compile errors

[rs2740 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80737

--- Comment #5 from TC  ---
(In reply to Tim Shen from comment #3)
> (In reply to TC from comment #1)
> > Looks like the constraint on the convert-everything constructor needs to
> > check for is_same, variant> first and short circuit if that's
> > true.
> 
> I'm not quite sure whether we need that short circuit - it's in a SFINAE
> environment, so if anything goes wrong, there shouldn't be a hard error.
> Therefore even if I agree that we should check is_same,
> variant> first, that shouldn't affect the correctness.

I'm not sure I agree. As http://stackoverflow.com/a/43963065/2756719 points
out, when you attempt to copy a variant, as part of overload resolution
the code attempts to check if you can convert a variant to any; that in
turn queries whether variant is copy constructible, and we are very close
to turtling all the way down.

It does seem like there might be a frontend problem here, though.

[Bug c++/80737] variant as class member resulting to compile errors

[timshen at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80737

--- Comment #4 from Tim Shen  ---
(In reply to Tim Shen from comment #3)
> (In reply to TC from comment #1)
> > Looks like the constraint on the convert-everything constructor needs to
> > check for is_same, variant> first and short circuit if that's
> > true.
> 
> I'm not quite sure whether we need that short circuit - it's in a SFINAE
> environment, so if anything goes wrong, there shouldn't be a hard error.
> Therefore even if I agree that we should check is_same,
> variant> first, that shouldn't affect the correctness.

s/even if/even though/.