change out async to a new name, with it being true by default.

antmor · antmor · commit 2ee9368732ea · 2025-09-24T18:08:31.000-07:00
add new unittest to test nullable lookup.
diff --git a/.runsettings b/.runsettings
@@ -13,7 +13,6 @@
     <ExecutionMode>Single</ExecutionMode>
     <FilenameFilter>(?i:Test)</FilenameFilter><!-- Regex filter -->
     <ForceListContent>true</ForceListContent>
-    <CombinedTimeout>30000</CombinedTimeout><!-- 30s in Milliseconds -->
     <DebugBreak>on</DebugBreak>
     <IncludeHidden>true</IncludeHidden>
     <Logging>Verbose</Logging>
diff --git a/cppwinrt/code_writers.h b/cppwinrt/code_writers.h
@@ -1881,7 +1881,7 @@ namespace cppwinrt
             upcall,
             bind<write_produce_args>(method_signature));
         w.write(R"(
-                if (out_param_val) 
+                if (out_param_val.has_value()) 
                 {
                     *% = detach_from<%>(std::move(*out_param_val));
                 }
@@ -1934,6 +1934,7 @@ namespace cppwinrt
         std::string upcall = "this->shim().";
         auto name = get_name(method);
         upcall += name;
+        // if (type_name == "Windows.Foundation.Collections.IMapView`2")
         if (name == "Lookup")
         {
             // Special-case Lookup to look for a TryLookup here, to avoid a throw/originate
diff --git a/strings/base_coroutine_foundation.h b/strings/base_coroutine_foundation.h
@@ -351,9 +351,9 @@ namespace winrt::impl
             return m_promise->enable_cancellation_propagation(value);
         }
 
-        bool avoid_cancel_origination(bool value = true) const noexcept
+        bool originate_on_cancel(bool value = true) const noexcept
         {
-            return m_promise->enable_avoid_cancel_origination(value);
+            return m_promise->originate_on_cancel(value);
         }
     private:
 
@@ -488,13 +488,13 @@ namespace winrt::impl
                 if (m_status.load(std::memory_order_relaxed) == AsyncStatus::Started)
                 {
                     m_status.store(AsyncStatus::Canceled, std::memory_order_relaxed);
-                    if (cancellable_promise::avoid_cancel_origination_enabled())
+                    if (cancellable_promise::originate_on_cancel())
                     {
-                        m_exception = std::make_exception_ptr(hresult_canceled(hresult_error::avoid_originate));
+                        m_exception = std::make_exception_ptr(hresult_canceled());
                     }
                     else
                     {
-                        m_exception = std::make_exception_ptr(hresult_canceled());
+                        m_exception = std::make_exception_ptr(hresult_canceled(hresult_error::no_originate));
                     }
                     cancel = std::move(m_cancel);
                 }
@@ -639,13 +639,13 @@ namespace winrt::impl
         {
             if (Status() == AsyncStatus::Canceled)
             {
-                if (cancellable_promise::avoid_cancel_origination_enabled())
+                if (cancellable_promise::originate_on_cancel())
                 {
-                    throw winrt::hresult_canceled(hresult_error::avoid_originate);
+                    throw winrt::hresult_canceled();
                 }
                 else
                 {
-                    throw winrt::hresult_canceled();
+                    throw winrt::hresult_canceled(hresult_error::no_originate);
                 }
             }
 
diff --git a/strings/base_coroutine_threadpool.h b/strings/base_coroutine_threadpool.h
@@ -180,14 +180,14 @@ WINRT_EXPORT namespace winrt
             return m_propagate_cancellation;
         }
 
-        bool enable_avoid_cancel_origination(bool value) noexcept
+        bool originate_on_cancel(bool value = true) noexcept
         {
-            return std::exchange(m_avoid_cancel_origination, value);
+            return std::exchange(m_originate_on_cancel, value);
         }
 
-        bool avoid_cancel_origination_enabled() const noexcept
+        bool should_originate_on_cancel() const noexcept
         {
-            return m_avoid_cancel_origination;
+            return m_originate_on_cancel;
         }
 
     private:
@@ -196,7 +196,7 @@ WINRT_EXPORT namespace winrt
         std::atomic<canceller_t> m_canceller{ nullptr };
         void* m_context{ nullptr };
         bool m_propagate_cancellation{ false };
-        bool m_avoid_cancel_origination{ false };
+        bool m_originate_on_cancel{ true }; // By default, will call RoOriginateError before throwing a cancel error code.
     };
 
     template <typename Derived>
diff --git a/strings/base_error.h b/strings/base_error.h
@@ -84,8 +84,8 @@ WINRT_EXPORT namespace winrt
 {
     struct hresult_error
     {
-        struct avoid_originate_t {};
-        static constexpr avoid_originate_t avoid_originate{};
+        struct no_originate_t {};
+        static constexpr no_originate_t no_originate{};
 
         using from_abi_t = take_ownership_from_abi_t;
         static constexpr auto from_abi{ take_ownership_from_abi };
@@ -112,7 +112,7 @@ WINRT_EXPORT namespace winrt
             originate(code, nullptr, sourceInformation);
         }
 
-        explicit hresult_error(hresult const code, avoid_originate_t, [[maybe_unused]] winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current()) noexcept : m_code(verify_error(code))
+        explicit hresult_error(hresult const code, no_originate_t, [[maybe_unused]] winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current()) noexcept : m_code(verify_error(code))
         {
         }
 
@@ -332,7 +332,7 @@ WINRT_EXPORT namespace winrt
     struct hresult_canceled : hresult_error
     {
         hresult_canceled(winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current()) noexcept : hresult_error(impl::error_canceled, sourceInformation) {}
-        hresult_canceled(hresult_error::avoid_originate_t) noexcept : hresult_error(impl::error_canceled, hresult_error::avoid_originate) {}
+        hresult_canceled(hresult_error::no_originate_t) noexcept : hresult_error(impl::error_canceled, hresult_error::no_originate) {}
         hresult_canceled(param::hstring const& message, winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current()) noexcept : hresult_error(impl::error_canceled, message, sourceInformation) {}
         hresult_canceled(take_ownership_from_abi_t, winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current()) noexcept : hresult_error(impl::error_canceled, take_ownership_from_abi, sourceInformation) {}
     };
diff --git a/test/old_tests/UnitTests/TryLookup.cpp b/test/old_tests/UnitTests/TryLookup.cpp
@@ -188,3 +188,58 @@ TEST_CASE("trylookup_from_abi specialization")
     // regular lookup should throw the same error
     REQUIRE_THROWS_AS(map.Lookup(123), hresult_wrong_thread);
 }
+
+TEST_CASE("trylookup_from_abi specialization with IInspectable")
+{
+    // A map that throws a specific error, used to verify various edge cases.
+    // and implements tryLookup, to take advantage of an optimization to avoid a throw. 
+    struct map_with_try_lookup : implements<map_with_try_lookup, IMapView<int, IInspectable>>
+    {
+        hresult codeToThrow{ S_OK };
+        bool shouldThrowOnTryLookup{ false };
+        bool returnNullptr{ false };
+        std::optional<IInspectable> TryLookup(int, trylookup_from_abi_t)
+        {
+            if (returnNullptr)
+            {
+                return { nullptr };
+            }
+            else if (shouldThrowOnTryLookup)
+            { 
+                throw_hresult(codeToThrow); 
+            }
+            else
+            {
+                return { std::nullopt };
+            }
+        }
+        IInspectable Lookup(int) { throw_hresult(E_UNEXPECTED); } // shouldn't be called by the test
+        int32_t Size() { throw_hresult(E_UNEXPECTED); } // shouldn't be called by the test
+        bool HasKey(int) { throw_hresult(E_UNEXPECTED); } // shouldn't be called by the test
+        void Split(IMapView<int, IInspectable>&, IMapView<int, IInspectable>&) { throw_hresult(E_UNEXPECTED); } // shouldn't be called by the test
+    };
+
+    auto self = make_self<map_with_try_lookup>();
+    IMapView<int, IInspectable> map = *self;
+
+    // Ensure that we return a value on nullptr, a nullptr is a valid IInspectable in the Map
+    self->returnNullptr = true;
+    REQUIRE(map.TryLookup(123) == IInspectable{nullptr});
+    REQUIRE(map.Lookup(123) == IInspectable{nullptr});
+    
+    // Make sure that we use the TryLookup specialization, and don't throw an unexpected exception.
+    self->shouldThrowOnTryLookup = false;
+    self->returnNullptr = false;
+    REQUIRE(map.TryLookup(123) == IInspectable{nullptr});
+    // make sure regular lookup stll throws bounds
+    REQUIRE_THROWS_AS(map.Lookup(123), hresult_out_of_bounds);
+
+    // Simulate a non-agile map that is being accessed from the wrong thread.
+    // "Try" operations should throw rather than erroneously report "not found".
+    // Because they didn't even try. The operation never got off the ground.
+    self->shouldThrowOnTryLookup = true;
+    self->codeToThrow = RPC_E_WRONG_THREAD;
+    REQUIRE_THROWS_AS(map.TryLookup(123), hresult_wrong_thread);
+    // regular lookup should throw the same error
+    REQUIRE_THROWS_AS(map.Lookup(123), hresult_wrong_thread);
+}
diff --git a/test/test/async_check_cancel.cpp b/test/test/async_check_cancel.cpp
@@ -133,7 +133,7 @@ namespace
         winrt_throw_hresult_handler = exceptionLogger;
 
         auto cancel = co_await get_cancellation_token();
-        cancel.avoid_cancel_origination(true);
+        cancel.originate_on_cancel(false);
 
         co_await resume_on_signal(event);
 

Original file line number	Diff line number	Diff line change
`@@ -1881,7 +1881,7 @@ namespace cppwinrt`
`1881`	`1881`	`upcall,`
`1882`	`1882`	`bind<write_produce_args>(method_signature));`
`1883`	`1883`	`w.write(R"(`
`1884`		`- if (out_param_val)`
	`1884`	`+ if (out_param_val.has_value())`
`1885`	`1885`	`{`
`1886`	`1886`	`% = detach_from<%>(std::move(out_param_val));`
`1887`	`1887`	`}`
`@@ -1934,6 +1934,7 @@ namespace cppwinrt`
`1934`	`1934`	`std::string upcall = "this->shim().";`
`1935`	`1935`	`auto name = get_name(method);`
`1936`	`1936`	`upcall += name;`
	`1937`	+ // if (type_name == "Windows.Foundation.Collections.IMapView`2")
`1937`	`1938`	`if (name == "Lookup")`
`1938`	`1939`	`{`
`1939`	`1940`	`// Special-case Lookup to look for a TryLookup here, to avoid a throw/originate`
Original file line number	Diff line number	Diff line change
`@@ -351,9 +351,9 @@ namespace winrt::impl`
`351`	`351`	`return m_promise->enable_cancellation_propagation(value);`
`352`	`352`	`}`
`353`	`353`
`354`		`- bool avoid_cancel_origination(bool value = true) const noexcept`
	`354`	`+ bool originate_on_cancel(bool value = true) const noexcept`
`355`	`355`	`{`
`356`		`- return m_promise->enable_avoid_cancel_origination(value);`
	`356`	`+ return m_promise->originate_on_cancel(value);`
`357`	`357`	`}`
`358`	`358`	`private:`
`359`	`359`
`@@ -488,13 +488,13 @@ namespace winrt::impl`
`488`	`488`	`if (m_status.load(std::memory_order_relaxed) == AsyncStatus::Started)`
`489`	`489`	`{`
`490`	`490`	`m_status.store(AsyncStatus::Canceled, std::memory_order_relaxed);`
`491`		`- if (cancellable_promise::avoid_cancel_origination_enabled())`
	`491`	`+ if (cancellable_promise::originate_on_cancel())`
`492`	`492`	`{`
`493`		`- m_exception = std::make_exception_ptr(hresult_canceled(hresult_error::avoid_originate));`
	`493`	`+ m_exception = std::make_exception_ptr(hresult_canceled());`
`494`	`494`	`}`
`495`	`495`	`else`
`496`	`496`	`{`
`497`		`- m_exception = std::make_exception_ptr(hresult_canceled());`
	`497`	`+ m_exception = std::make_exception_ptr(hresult_canceled(hresult_error::no_originate));`
`498`	`498`	`}`
`499`	`499`	`cancel = std::move(m_cancel);`
`500`	`500`	`}`
`@@ -639,13 +639,13 @@ namespace winrt::impl`
`639`	`639`	`{`
`640`	`640`	`if (Status() == AsyncStatus::Canceled)`
`641`	`641`	`{`
`642`		`- if (cancellable_promise::avoid_cancel_origination_enabled())`
	`642`	`+ if (cancellable_promise::originate_on_cancel())`
`643`	`643`	`{`
`644`		`- throw winrt::hresult_canceled(hresult_error::avoid_originate);`
	`644`	`+ throw winrt::hresult_canceled();`
`645`	`645`	`}`
`646`	`646`	`else`
`647`	`647`	`{`
`648`		`- throw winrt::hresult_canceled();`
	`648`	`+ throw winrt::hresult_canceled(hresult_error::no_originate);`
`649`	`649`	`}`
`650`	`650`	`}`
`651`	`651`
Original file line number	Diff line number	Diff line change
`@@ -180,14 +180,14 @@ WINRT_EXPORT namespace winrt`
`180`	`180`	`return m_propagate_cancellation;`
`181`	`181`	`}`
`182`	`182`
`183`		`- bool enable_avoid_cancel_origination(bool value) noexcept`
	`183`	`+ bool originate_on_cancel(bool value = true) noexcept`
`184`	`184`	`{`
`185`		`- return std::exchange(m_avoid_cancel_origination, value);`
	`185`	`+ return std::exchange(m_originate_on_cancel, value);`
`186`	`186`	`}`
`187`	`187`
`188`		`- bool avoid_cancel_origination_enabled() const noexcept`
	`188`	`+ bool should_originate_on_cancel() const noexcept`
`189`	`189`	`{`
`190`		`- return m_avoid_cancel_origination;`
	`190`	`+ return m_originate_on_cancel;`
`191`	`191`	`}`
`192`	`192`
`193`	`193`	`private:`
`@@ -196,7 +196,7 @@ WINRT_EXPORT namespace winrt`
`196`	`196`	`std::atomic<canceller_t> m_canceller{ nullptr };`
`197`	`197`	`void* m_context{ nullptr };`
`198`	`198`	`bool m_propagate_cancellation{ false };`
`199`		`- bool m_avoid_cancel_origination{ false };`
	`199`	`+ bool m_originate_on_cancel{ true }; // By default, will call RoOriginateError before throwing a cancel error code.`
`200`	`200`	`};`
`201`	`201`
`202`	`202`	`template <typename Derived>`
Original file line number	Diff line number	Diff line change
`@@ -84,8 +84,8 @@ WINRT_EXPORT namespace winrt`
`84`	`84`	`{`
`85`	`85`	`struct hresult_error`
`86`	`86`	`{`
`87`		`- struct avoid_originate_t {};`
`88`		`- static constexpr avoid_originate_t avoid_originate{};`
	`87`	`+ struct no_originate_t {};`
	`88`	`+ static constexpr no_originate_t no_originate{};`
`89`	`89`
`90`	`90`	`using from_abi_t = take_ownership_from_abi_t;`
`91`	`91`	`static constexpr auto from_abi{ take_ownership_from_abi };`
`@@ -112,7 +112,7 @@ WINRT_EXPORT namespace winrt`
`112`	`112`	`originate(code, nullptr, sourceInformation);`
`113`	`113`	`}`
`114`	`114`
`115`		`- explicit hresult_error(hresult const code, avoid_originate_t, [[maybe_unused]] winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current()) noexcept : m_code(verify_error(code))`
	`115`	`+ explicit hresult_error(hresult const code, no_originate_t, [[maybe_unused]] winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current()) noexcept : m_code(verify_error(code))`
`116`	`116`	`{`
`117`	`117`	`}`
`118`	`118`
`@@ -332,7 +332,7 @@ WINRT_EXPORT namespace winrt`
`332`	`332`	`struct hresult_canceled : hresult_error`
`333`	`333`	`{`
`334`	`334`	`hresult_canceled(winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current()) noexcept : hresult_error(impl::error_canceled, sourceInformation) {}`
`335`		`- hresult_canceled(hresult_error::avoid_originate_t) noexcept : hresult_error(impl::error_canceled, hresult_error::avoid_originate) {}`
	`335`	`+ hresult_canceled(hresult_error::no_originate_t) noexcept : hresult_error(impl::error_canceled, hresult_error::no_originate) {}`
`336`	`336`	`hresult_canceled(param::hstring const& message, winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current()) noexcept : hresult_error(impl::error_canceled, message, sourceInformation) {}`
`337`	`337`	`hresult_canceled(take_ownership_from_abi_t, winrt::impl::slim_source_location const& sourceInformation = winrt::impl::slim_source_location::current()) noexcept : hresult_error(impl::error_canceled, take_ownership_from_abi, sourceInformation) {}`
`338`	`338`	`};`