Add a hook to throw_hresult for integration with WIL #843
Conversation
|
Thanks! |
|
I've queued a new build with your change: 2.0.210122.3 |
| { | ||
| if (winrt_throw_hresult_handler) | ||
| { | ||
| winrt_throw_hresult_handler(0, nullptr, nullptr, _ReturnAddress(), result); |
There was a problem hiding this comment.
_ReturnAddress is a MSVC extension. For portability, may need to say
void* returnAddress = nullptr;
#if defined(_MSC_VER)
returnAddress = _ReturnAddress();
#elif defined(__GNUC__) // also detects clang, icc, djgpp, ellcc, zapcc
returnAddress = __builtin_extract_return_addr(__builtin_return_address(0));
#endifFurthermore, you need to #include <intrin.h> before you can use _ReturnAddress(), but this code appears to be relying on the kindness of strangers. Probably better to include it explicitly.
There was a problem hiding this comment.
to_hresult was already using _ReturnAddress but yes not very portable...
There was a problem hiding this comment.
I was following the pattern established by the similar to_hresult extension point with the expectation that it was already correct/acceptable.
(For what it's worth the MSVC version of your snippet is a lot more readable than the else block :))
#841
Why is this change being made?
This change is being made to improve the interop between cppwinrt and WIL (particularly WIL's logging abilities for errors). It introduces a hook to the
winrt::throw_hresultmethod that <wil/cppwinrt.h> will be able to fill in with a valid function pointer. That function pointer will allow WIL's logging abilities to gain knowledge of the exceptions that are originated within cppwinrt.Briefly summarize what changed
An extern function pointer was created which is empty by default. If it is set then
winrt::throw_hresultwill call it before continuing with actually throwing the exception.One interesting note is that several of the parameters to
winrt_throw_hresult_handlerare not actually filled in. There is an expectation that C++20 source_location will allow correct file/function/line annotation to these calls for optimum debuggability. That hopefully isn't too far off in the future so the contract is being established now to avoid the need to change it soon in a backwards-compatible way. Once source_location does come online there will be no changes needed on the WIL side to log them properly.How was the change tested?
build_test_all.cmdwas used for this. A new test case was added to confirm that throw_hresult does call the handler if set. I also copied my private build of cppwinrt.exe to a test project to confirm that the generated base.h matches expectations.