try_as casts should not store COM error context; consume method cast checking should use return codes directly (#1450)

Why is this change being made?
I have been trying to ingest the HEAD of cppwinrt for some large internal projects and one of them had some test failures with the new version. The failures are because there is a try_as cast in their code that fails and is handled fine, but it leaves a COM error context floating around on that thread. Subsequent code fails to originate an error because it sees a context already active on the thread and NOOP'ed.

What this boils down to is that try_as should not have a behavioral change to store context when the cast fails.

Briefly summarize what changed
To address this problem I am taking a PR suggestion from @oldnewthing to have a new try_as_with_reason method that returns both the cast result as well as the HRESULT. The error context logic was only there to smuggle the HRESULT out of a call without an HRESULT return value, so if it is a direct return we don't need that anymore. The new method directly returns the HRESULT so there is no ambiguity.

The previous approach relied on a cast operator to call try_as (or just addref when the type is already a match). Thanks to the recent if constexpr code gen change those cases are now separated out. We have a code block where we know a cast is needed so try_as_with_reason can be called unconditionally. The code path where no cast is needed already circumvents this and is nicely unaffected.

This also made check_cast_result equiavelnt to check_hresult so it was deleted in favor of just calling check_hresult.

How was this change tested?
I did local builds of cppwinrt (both Debug and Release) and ran the tests. I am also compiling some large internal projects with it to ensure that there are no obvious breaks or regressions. I am expecting minimal to no binary size impact from this change.

Author: dmachaj

cppwinrt/code_writers.h

@@ -1130,16 +1130,16 @@ namespace cppwinrt
             {
                 // we intentionally ignore errors when unregistering event handlers to be consistent with event_revoker
                 //
-                // The `noexcept` versions will crash if check_cast_result throws but that is no different than previous
+                // The `noexcept` versions will crash if check_hresult throws but that is no different than previous
                 // behavior where it would not check the cast result and nullptr crash.  At least the exception will terminate
                 // immediately while preserving the error code and local variables.
                 format = R"(    template <typename D%> auto consume_%<D%>::%(%) const noexcept
     {%
         if constexpr (!std::is_same_v<D, %>)
         {
-            auto const& castedResult = static_cast<% const&>(static_cast<D const&>(*this));
+            auto const [castedResult, code] = impl::try_as_with_reason<%, D const*>(static_cast<D const*>(this));
+            check_hresult(code);
             auto const abiType = *(abi_t<%>**)&castedResult;
-            check_cast_result(abiType);
             abiType->%(%);
         }
         else
@@ -1156,9 +1156,9 @@ namespace cppwinrt
     {%
         if constexpr (!std::is_same_v<D, %>)
         {
-            auto const& castedResult = static_cast<% const&>(static_cast<D const&>(*this));
+            auto const [castedResult, code] = impl::try_as_with_reason<%, D const*>(static_cast<D const*>(this));
+            check_hresult(code);
             auto const abiType = *(abi_t<%>**)&castedResult;
-            check_cast_result(abiType);
             WINRT_VERIFY_(0, abiType->%(%));
         }
         else
@@ -1176,9 +1176,9 @@ namespace cppwinrt
     {%
         if constexpr (!std::is_same_v<D, %>)
         {
-            auto const& castedResult = static_cast<% const&>(static_cast<D const&>(*this));
+            auto const [castedResult, code] = impl::try_as_with_reason<%, D const*>(static_cast<D const*>(this));
+            check_hresult(code);
             auto const abiType = *(abi_t<%>**)&castedResult;
-            check_cast_result(abiType);
             check_hresult(abiType->%(%));
         }
         else

strings/base_error.h

@@ -538,27 +538,6 @@ namespace winrt::impl
         }
         return result;
     }
-
-    inline WINRT_IMPL_NOINLINE void check_cast_result(void* from, winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current())
-    {
-        if (!from)
-        {
-            com_ptr<impl::IRestrictedErrorInfo> restrictedError;
-            if (WINRT_IMPL_GetRestrictedErrorInfo(restrictedError.put_void()) == 0)
-            {
-                WINRT_IMPL_SetRestrictedErrorInfo(restrictedError.get());
-
-                int32_t code;
-                impl::bstr_handle description;
-                impl::bstr_handle restrictedDescription;
-                impl::bstr_handle capabilitySid;
-                if (restrictedError->GetErrorDetails(description.put(), &code, restrictedDescription.put(), capabilitySid.put()) == 0)
-                {
-                    throw hresult_error(code, take_ownership_from_abi, sourceInformation);
-                }
-            }
-        }
-    }
 }
 
 #undef WINRT_IMPL_RETURNADDRESS

strings/base_extern.h

@@ -32,8 +32,6 @@ extern "C"
     int32_t __stdcall WINRT_IMPL_RoCaptureErrorContext(int32_t error) noexcept WINRT_IMPL_LINK(RoCaptureErrorContext, 4);
     void __stdcall WINRT_IMPL_RoFailFastWithErrorContext(int32_t) noexcept WINRT_IMPL_LINK(RoFailFastWithErrorContext, 4);
     int32_t __stdcall WINRT_IMPL_RoTransformError(int32_t, int32_t, void*) noexcept WINRT_IMPL_LINK(RoTransformError, 12);
-    int32_t __stdcall WINRT_IMPL_GetRestrictedErrorInfo(void**) noexcept WINRT_IMPL_LINK(GetRestrictedErrorInfo, 4);
-    int32_t __stdcall WINRT_IMPL_SetRestrictedErrorInfo(void*) noexcept WINRT_IMPL_LINK(SetRestrictedErrorInfo, 4);
 
     void* __stdcall WINRT_IMPL_LoadLibraryExW(wchar_t const* name, void* unused, uint32_t flags) noexcept WINRT_IMPL_LINK(LoadLibraryExW, 12);
     int32_t __stdcall WINRT_IMPL_FreeLibrary(void* library) noexcept WINRT_IMPL_LINK(FreeLibrary, 4);

strings/base_implements.h

@@ -792,9 +792,20 @@ namespace winrt::impl
         {
             return m_inner.operator bool();
         }
+
+        template <typename To, typename From>
+        friend auto winrt::impl::try_as_with_reason(From ptr) noexcept;
+
     protected:
         static constexpr bool is_composing = true;
         Windows::Foundation::IInspectable m_inner;
+
+    private:
+        template <typename Qi>
+        auto try_as_with_reason() const noexcept
+        {
+            return m_inner.try_as_with_reason<Qi>();
+        }
     };
 
     template <typename D, bool>

strings/base_windows.h

@@ -128,12 +128,31 @@ namespace winrt::impl
         }
 
         void* result{};
-        hresult code = ptr->QueryInterface(guid_of<To>(), &result);
-        if (code < 0)
+        ptr->QueryInterface(guid_of<To>(), &result);
+        return wrap_as_result<To>(result);
+    }
+
+    template <typename To, typename From, std::enable_if_t<is_com_interface_v<To>, int> = 0>
+    std::pair<com_ref<To>, hresult> try_as_with_reason(From* ptr) noexcept
+    {
+#ifdef WINRT_DIAGNOSTICS
+        get_diagnostics_info().add_query<To>();
+#endif
+
+        if (!ptr)
         {
-            WINRT_IMPL_RoCaptureErrorContext(code);
+            return { nullptr, 0 };
         }
-        return wrap_as_result<To>(result);
+
+        void* result{};
+        hresult code = ptr->QueryInterface(guid_of<To>(), &result);
+        return { wrap_as_result<To>(result), code };
+    }
+
+    template <typename To, typename From>
+    auto try_as_with_reason(From ptr) noexcept
+    {
+        return ptr->template try_as_with_reason<To>();
     }
 }
 
@@ -209,6 +228,12 @@ WINRT_EXPORT namespace winrt::Windows::Foundation
             return impl::try_as<To>(m_ptr);
         }
 
+        template <typename To>
+        auto try_as_with_reason() const noexcept
+        {
+            return impl::try_as_with_reason<To>(m_ptr);
+        }
+
         template <typename To>
         void as(To& to) const
         {