Stl: <memory>: enable_shared_from_this could be more helpful

[oldnewthing]

Describe the bug

enable_shared_from_this must be a unique public base class in order for it to work. Suggest issuing a diagnostic if the prerequisites are not met, to avoid developers scratching their heads trying to figure out why their call to shared_from_this() is crashing.

Command-line test case

C:\Temp>type repro.cpp
#include <memory>

class S : std::enable_shared_from_this<S>
{
    public: auto meow() { return shared_from_this(); }
};

int main()
{
    auto s = std::make_shared<S>();
    s->meow(); // throws bad_weak_ptr
}

C:\Temp>cl /EHsc /W4 /WX .\repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.25.28611 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

repro.cpp
Microsoft (R) Incremental Linker Version 14.25.28611.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:repro.exe
repro.obj

C:\Temp>.\repro.exe

C:\Temp>echo %ERRORLEVEL%
-1073740791

Expected behavior
Diagnostic informing you that "you're holding it wrong".

STL version

• Microsoft Visual Studio Community 2019
  Version 16.5.1

Additional context
Possible solution:

template <class _Ty>
class enable_shared_from_this { // provide member functions that create shared_ptr to this
public:
    using _Esft_type = enable_shared_from_this;

    _NODISCARD shared_ptr<_Ty> shared_from_this() {
>>>>    if constexpr (!_Can_enable_shared<_Ty>::value) {
>>>>        _Warning_enable_shared_from_this_must_be_unambiguous_public_base_class();
>>>>    }
        return shared_ptr<_Ty>(_Wptr);
    }

>>>>[[deprecated]] void _Warning_enable_shared_from_this_must_be_unambiguous_public_base_class() {}

…
}

(Similarly for weak_from_this.)

[BillyONeal]

We can indeed generate diagnostics by referencing [[deprecated]] things which could help this scenario, but I'm not positive myself whether we can detect the condition here because I don't know whether _Ty needs to be complete (it certainly isn't complete when enable_shared_from_this is instantiated, but it probably is when shared_from_this is....).

I hope folks who understand the requirements here better can respond.

[nliber]

You need to publicly inherit from shared_from_this in order for it to work.

[oldnewthing]

@nliber Right. The point is that there is no diagnostic when you make this mistake.

enable_shared_from_this permits incomplete types (necessarily, since it is instantiated before the parent is complete), so this code is technically legal:

class S; // incomplete
void f(std::enable_shared_from_this<S>& weird) { weird.shared_from_this(); }

and the compiler has to cope with an incomplete template type parameter. Sigh.

[CaseyCarter]

As @oldnewthing observes, there are well-formed programs which instantiate enable_shared_from_this<T>::shared_from_this while T is incomplete, so it doesn't seem like there's anything the library can do to help here. A compiler or static analyzer, however, could easily warn when it observes a class that non-publicly derives from a specialization of std::enable_shared_from_this. I suggest you file a feature-request suggestion at https://developercommunity.visualstudio.com/content/idea/post.html?space=62.

[AlexGuteniev]

I tried to look for a is_incomplete type traits, and found @oldnewthing's posts:
https://devblogs.microsoft.com/oldnewthing/20190708-00/?p=102664
https://devblogs.microsoft.com/oldnewthing/20190709-00/?p=102671
https://devblogs.microsoft.com/oldnewthing/20190710-00/?p=102678
https://devblogs.microsoft.com/oldnewthing/20190711-00/?p=102682
https://devblogs.microsoft.com/oldnewthing/20190712-00/?p=102690

So skipping the assert for incompletes can be made.

Ant ultimately the assert, which is skipped for incompletes, will be triggered, as in the header where class S : std::enable_shared_from_this<S> is written, S is complete.

[oldnewthing]

@AlexGuteniev Suppose the method can be called twice, with the type incomplete at the first call, but then becomes complete, and then is called again. This causes the two instantiations of the template to disagree, which in turn causes the content of an inline function to change, and I believe both of those scenarios constitute IFNDR.

[oldnewthing]

Converted to compiler/code analysis feature request, since nothing the library can do, due to the need to support incomplete types.

[AlexGuteniev]

It looks like then that the ability to emit arbitrary warnings from template instantiation would be the best way to implement it.

Currently, STL can only emit deprecation warning via [[depreacted]] attribute.

There may be a sense to emit other warnings.
(Like what I encountered recently, passing toatomic_ref such T that is not aligned at required_alignment)