[libcxx-commits] [libcxx] [libc++] Re-introduce special support for narrowing conversions to bool in variant (PR #73121)

[David Benjamin via libcxx-commits]

davidben wrote:

This is all a bit of a mess. :-) Here's what I think is going on. (NB: I may have some details wrong here.)

As I understand it, there are four different `std::variant` behaviors circling around here:

* Originally, `variant<A, B>`'s constructor was modeled as having overloads for `A` and `B`. If your type implicitly converted to `A`, it would bind to `A`. If the overload set was ambiguous, you can't construct the variant. Let's call this behavior *pre-p0608r3*.

* Then [p0608r3](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0608r3.html) removed overloads for narrowing conversions, but with a special quirk for `bool.` Let's call this behavior *p0608r3*.

* When libc++ implemented this, it broke things, so we added a [flag](https://github.com/llvm/llvm-project/commit/128af315957eb1516c3b7ffd389774a0287ba300) to restore the old behavior. However, that didn't quite restore the old behavior because it intentionally kept the boolean special case:

  > For this reasons, I am adding a flag to disable the narrowing conversion changes but not the boolean conversions one

  Let's call this behavior *mostly-pre-p0608r3*. Keeping the `bool` special case has the side effect of making some cases that would be ambiguous overloads unambiguous.

* Then [p1957r2](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1957r2.html) removed the boolean special case. Let's call this behavior *p1957r2*. This is the correct behavior.

Prior to my PR, `absl::variant` was (as far as I can tell) *pre-p0608r3*. libc++ default was *p0608r3*. libc++ with flag was *mostly-pre-p0608r3*.

#71502 changed libc++'s defaults from *p0608r3* to *p1957r2*, the correct behavior. The motivation on my end was Chrome (which doesn't plan to set the flag) ran into the `vector<bool>` issue in #62332. It inadvertently also turned libc++ with flag from *mostly-pre-p0608r3* back into *pre-p0608r3*.

The delta from *mostly-pre-p0608r3* back into *pre-p0608r3* seems to be a bit turbulent. The delta from *p0608r3* to *p1957r2* seems to be fine? At least, we haven't seen any evidence yet that those changes cause much problems. The causes that hit the two "pre" cases are mostly all rejected by the narrowing rule anyway.

Since that's a lot of text, here are some tables to summarize the mess. :-)

|                                            |pre-p0608r3                |mostly-pre-p0608r3                  |p0608r3          |p1957r2          |
|--------------------------------------------|---------------------------|------------------------------------|-----------------|-----------------|
|allows narrowing                            |Y                          |Y                                   |N                |N                |
|`bool` must match exactly                   |N                          |Y                                   |Y                |N                |
|`variant<string, bool> = "abc"`             |`bool` :-(                 |`string`                            |`string`         |`string`         |
|`variant<bool, int> = vector<bool>{true}[0]`|`bool`                     |`int` :-(                           |`int` :-(        |`bool`           |
|`variant<float, string> = 0.0`              |`float`                    |`float`                             |error (narrowing)|error (narrowing)|
|`variant<float, bool> = 0.0`                |error (ambiguous overloads)|`float` (`bool` overload suppressed)|error (narrowing)|error (narrowing)|

|impl                  |behavior          |
|----------------------|------------------|
|absl                  |pre-p0608r3       |
|libc++ 17 default     |p0608r3           |
|libc++ 17 with flag   |mostly-pre-p0608r3|
|libc++ trunk default  |p1957r2           |
|libc++ trunk with flag|pre-p0608r3       |
|libstdc++             |p1957r2           |



https://github.com/llvm/llvm-project/pull/73121