weak_ptr conversions can sometimes avoid locking

[Alex Guteniev (AlexGuteniev)]

Resolves #258

Test happen to recreate the issue for me on attempt #0
if unconditionally avoid locking

[S. B. Tam (cpplearner)]

IIUC this does not optimize for the X -> const X case mentioned in #258 (since static_cast(declval()) is invalid). Maybe also consider is_convertible<_Ty2(*)[], _Ty(*)[]>?

[Alex Guteniev (AlexGuteniev)]

IIUC this does not optimize for the X -> const X case mentioned in #258 (since static_cast(declval()) is invalid). Maybe also consider is_convertible<_Ty2(*)[], _Ty(*)[]>?

Good point, but I preferred to update the existing check to cover Derived -> const Base as well

[Stephan T. Lavavej (StephanTLavavej)]

S. B. Tam (@cpplearner) I think that Alex Guteniev (@AlexGuteniev)'s code does handle X -> const X, although (as I mentioned in my review) the logic is very subtle.

Suppose that we're constructing a weak_ptr from weak_ptr. The constructor is weak_ptr(const weak_ptr<_Ty2>& _Other), so "our" _Ty is const int and "their" _Ty2 is int. The type trait's static_cast is intentionally reversed:

template <class _Ty2>
static constexpr bool _Is_virtual_base_cast<_Ty2, decltype(static_cast<const _Ty2*>(_STD declval<_Ty*>()))> = false;

This starts with declval<_Ty*>(), i.e. declval(), and asks whether we can static_cast, i.e. static_cast. So we've formed the same type, the static_cast is well-formed, and the type trait returns false (saying "we don't need the expensive machinery to avoid virtual inheritance weirdness").

At a high level, the technique is saying that when implicitly converting from source-to-dest (whether adding constness or converting derived-to-base), a static_cast from dest-to-const source is valid except in the virtual inheritance case we care about.

(Aside: I noticed that this doesn't consider volatile and I'm fine with that, it'll select the slow path.)

[Alex Guteniev (AlexGuteniev)]

For the record, S. B. Tam (@cpplearner) comment was useful, as I added const after it

[Stephan T. Lavavej (StephanTLavavej)]

Thanks for finding the EDG bug! I reported it as DevCom-1564433 and pushed a perma-workaround after discovering that changing _STD declval<_Ty*>() to static_cast<_Ty*>(nullptr) avoids the bug.

[Stephan T. Lavavej (StephanTLavavej)]

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

[Stephan T. Lavavej (StephanTLavavej)]

Alex Guteniev (@AlexGuteniev) Casey Carter (@CaseyCarter) I've pushed changes to fix test failures for /clr:pure, the Betrayer of Hope - the simplest thing is to entirely disable this optimization.

[Casey Carter (CaseyCarter)]

Alex Guteniev (@AlexGuteniev) Casey Carter (@CaseyCarter) I've pushed changes to fix test failures for /clr:pure, the Betrayer of Hope - the simplest thing is to entirely disable this optimization.

Having recently begun my fourth trip through the Wheel of Time, I'm deeply offended that you would liken /clr:pure to Ishamael. Not even one of the Forsaken is as dark and twisted as /clr:pure!

[Stephan T. Lavavej (StephanTLavavej)]

Thanks for implementing this long-dreamed-of optimization! 🐈 💤 😻