Skip to content

Reliably diagnose stack allocations in C++/WinRT#329

Merged
kennykerr merged 5 commits into
masterfrom
kennykerr-make
Apr 19, 2019
Merged

Reliably diagnose stack allocations in C++/WinRT#329
kennykerr merged 5 commits into
masterfrom
kennykerr-make

Conversation

@kennykerr

Copy link
Copy Markdown
Contributor

I previously introduced a runtime assertion to help diagnose and catch stack allocation bugs in C++/WinRT. Unfortunately, there are just too many cases where this does not catch stack allocations. Notably, if the local is created within a coroutine frame then it won't detect it since the coroutine frame is itself allocated on the heap. This update turns this runtime assertion into a compiler error that should catch such mistakes in all cases.

struct Sample : implements<Sample, IStringable>
{
    hstring ToString() { return L"Sample"; }
};

IAsyncOperation<hstring> Async()
{
    co_await resume_background();
    IStringable s = Sample(); // <-- Oops!
    co_return s.ToString();
}
error C2259: 'Sample': cannot instantiate abstract class 
  ... use_make_function_to_create_this_object ...

Double clicking on the error brings the developer to this helpful comment:

// Please use winrt::make<T>(args...) to avoid allocating a C++/WinRT implementation type on the stack.
virtual void use_make_function_to_create_this_object() = 0;

This does mean that private destructors aren't supported, but that's already a requirement if you want to use the final_release feature (deferred destruction) and the only reason you'd want a private destructor is to prevent someone from accidentally calling delete ptr and C++/WinRT makes that quite hard to do since you really have to go out of your way to get hold of a raw pointer in C++/WinRT. C++/WinRT then also ensures that all implementations are final, which provides the C++ optimizer the possibility to devirtualize further.

@DefaultRyan

Copy link
Copy Markdown
Member

Port to cppx

@kennykerr

Copy link
Copy Markdown
Contributor Author

I have a few things lined up where cppwinrt and cppx have diverged and I'm preparing a PR to bring them all in.

}

#ifdef _DEBUG
void use_make_function_to_create_this_object() final

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we issue a static_assert and more friendly message instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly I haven't found a way to express this with a static_assert.

@kennykerr kennykerr merged commit 8963736 into master Apr 19, 2019
@kennykerr kennykerr deleted the kennykerr-make branch April 22, 2019 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants