UiaBool conversion to bool is incorrect

[joemmett]

I'm a dev on the MSVC compiler team and discovered a problem with the implementation of UiaBool while attempting to fix an overload resolution bug in the compiler.

Here's a reduced sample based on a subset of UiaBool that illustrates the problem: https://godbolt.org/z/57nMx5

For the test of a UiaBool with an implicit conversion to bool MSVC currently (but incorrectly) calls UiaBool::operator bool(). All other compilers, and MSVC with my fix, instead call UiaTypeBase::operator LocalT*, which will always yield 'true' as it's returning the non-null address of the variant member.

The main problem here is that the operator bool() overload will be a worse conversion than the base LocalT* conversion if the UiaBool object is not const (it involves a qualification conversion for the implicit object parameter, while the LocalT* conversion does not). This is because operator bool() is declared const, but operator LocalT*() is not.

There is a fundamental ambiguity here where there are implicit conversion sequences to both bool and BOOL*. One potential fix for this would be to make UiaBool::operator BOOL*() explicit.

[beweedon]

Thanks so much for bringing this to our attention! And thanks for the super clear minimal example!

We can probably address this within the next week or two (unless you want to open a PR 😄). Is the MSVC bug currently in public builds of the compiler, or was it regressed internally? Is two weeks a quick enough timeframe to implement our fix here before the MSVC fix make it to insider builds?

Thanks again!

[joemmett]

There's definitely no rush here. I found the problem with internal testing so the fix is not yet even in our development branch. Since this can be a silent behavior change it's likely I'll only be able to make this fix under /permissive- so you won't see it yet until #2 is resolved, but I wanted to give you a heads up about the potential problem.

Since I'm not very familiar with this project I didn't submit a PR cause I don't really know what the best solution is or how breaking it would be to existing users to change the BOOL* conversion to be explicit. There are several options to fix this, but each with tradeoffs. I'm happy to discuss more if you run into any problems trying to find a solution.