weak_ptr conversions can sometimes avoid locking

[AlexGuteniev]

Resolves #258

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

[cpplearner]

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

[AlexGuteniev]

IIUC this does not optimize for the X -> const X case mentioned in #258 (since static_cast<X*>(declval<const X*>()) 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

[StephanTLavavej]

@cpplearner I think that @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<const int> from weak_ptr<int>. 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<const int*>(), and asks whether we can static_cast<const _Ty2*>, i.e. static_cast<const int*>. 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.)

[AlexGuteniev]

For the record, @cpplearner comment was useful, as I added const after it

[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.

[StephanTLavavej]

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

[StephanTLavavej]

@AlexGuteniev @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.

[CaseyCarter]

@AlexGuteniev @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!

[StephanTLavavej]

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