Address hresult comments by adding tests and removing source_locatoin, special-cased further by checking the typename.

Author: antmor

cppwinrt/code_writers.h

@@ -1903,7 +1903,7 @@ namespace cppwinrt
         }
     }
 
-    static void write_produce_method(writer& w, MethodDef const& method)
+    static void write_produce_method(writer& w, MethodDef const& method, TypeDef const& type)
     {
         std::string_view format;
 
@@ -1934,11 +1934,12 @@ 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")
+
+        auto typeName = type.TypeName();
+        if (((typeName == "IMapView`2") || (typeName == "IMap`2"))
+            && (name == "Lookup"))
         {
-            // Special-case Lookup to look for a TryLookup here, to avoid a throw/originate
-            // and add a small performance improvement.
+            // Special-case IMap*::Lookup to look for a TryLookup here, to avoid extranous throw/originates
             std::string tryLookupUpCall = "this->shim().TryLookup";
             format = R"(        int32_t __stdcall %(%) noexcept final try
         {
@@ -2012,7 +2013,7 @@ namespace cppwinrt
                 break;
             }
 
-            w.write_each<write_produce_method>(info.type.MethodList());
+            w.write_each<write_produce_method>(info.type.MethodList(), info.type);
         }
     }
 
@@ -2034,7 +2035,7 @@ namespace cppwinrt
             bind<write_comma_generic_typenames>(generics),
             type,
             type,
-            bind_each<write_produce_method>(type.MethodList()),
+            bind_each<write_produce_method>(type.MethodList(), type),
             bind<write_fast_produce_methods>(type));
     }
 

strings/base_collections_base.h

@@ -514,10 +514,10 @@ WINRT_EXPORT namespace winrt
 
             if (pair == static_cast<D const&>(*this).get_container().end())
             {
-                return { std::nullopt };
+                return std::nullopt;
             }
 
-            return { static_cast<D const&>(*this).unwrap_value(pair->second) };
+            return static_cast<D const&>(*this).unwrap_value(pair->second);
         }
 
         V Lookup(K const& key) const

strings/base_error.h

@@ -112,7 +112,7 @@ WINRT_EXPORT namespace winrt
             originate(code, nullptr, sourceInformation);
         }
 
-        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))
+        explicit hresult_error(hresult const code, no_originate_t) noexcept : m_code(verify_error(code))
         {
         }
 

strings/base_meta.h

@@ -308,9 +308,7 @@ namespace winrt::impl
     {
         template <typename U, typename = decltype(std::declval<U>().TryLookup(std::declval<K>(), trylookup_from_abi))> static constexpr bool get_value(int) { return true; }
         template <typename> static constexpr bool get_value(...) { return false; }
-
     public:
-
         static constexpr bool value = get_value<D>(0);
     };
 

test/test_cpp20/custom_error.cpp

@@ -37,9 +37,9 @@ namespace
 
 #if defined(_LIBCPP_VERSION) && _LIBCPP_VERSION < 170000
 // <source_location> not available in libc++ before LLVM 16
-TEST_CASE("custom_error_logger", "[!shouldfail]")
+TEST_CASE("custom_error_logger_on_throw", "[!shouldfail]")
 #else
-TEST_CASE("custom_error_logger")
+TEST_CASE("custom_error_logger_on_throw")
 #endif
 {
     // Set up global handler
@@ -72,3 +72,62 @@ TEST_CASE("custom_error_logger")
     winrt_throw_hresult_handler = nullptr;
     s_loggerCalled = false;
 }
+template<typename... Args>
+void HresultOnLine80(Args... args)
+{
+    // Validate that handler translated on creating an HRESULT
+#line 80 // Force next line to be reported as line number 80
+    winrt::hresult_canceled(std::forward<Args>(args)...);
+}
+
+#if defined(_LIBCPP_VERSION) && _LIBCPP_VERSION < 170000
+// <source_location> not available in libc++ before LLVM 16
+TEST_CASE("custom_error_logger_on_originate", "[!shouldfail]")
+#else
+TEST_CASE("custom_error_logger_on_originate")
+#endif
+{
+    // Set up global handler
+    REQUIRE(!s_loggerCalled);
+    REQUIRE(!winrt_throw_hresult_handler);
+    winrt_throw_hresult_handler = logger;
+
+    HresultOnLine80();
+    REQUIRE(s_loggerCalled);
+    // In C++20 these fields should be filled in by std::source_location
+    REQUIRE(s_loggerArgs.lineNumber == 80);
+    const auto fileNameSv = std::string_view(s_loggerArgs.fileName);
+    REQUIRE(!fileNameSv.empty());
+    REQUIRE(fileNameSv.find("custom_error.cpp") != std::string::npos);
+#ifdef _DEBUG
+    const auto functionNameSv = std::string_view(s_loggerArgs.functionName);
+    REQUIRE(!functionNameSv.empty());
+    // Every compiler has a slightly different naming approach for this function, and even the same
+    // compiler can change its mind over time.  Instead of matching the entire function name just
+    // match against the part we care about.
+    REQUIRE((functionNameSv.find("HresultOnLine80") != std::string_view::npos));
+#else
+    REQUIRE(s_loggerArgs.functionName == nullptr);
+#endif // _DEBUG
+
+    REQUIRE(s_loggerArgs.returnAddress);
+    REQUIRE(s_loggerArgs.result == HRESULT_FROM_WIN32(ERROR_CANCELLED)); // E_ILLEGAL_DELEGATE_ASSIGNMENT)
+
+    s_loggerCalled = false;
+    s_loggerArgs.lineNumber = 0;
+    // verify HRESULT with a custom message
+    HresultOnLine80(L"with custom message");
+    REQUIRE(s_loggerCalled);
+    REQUIRE(s_loggerArgs.lineNumber == 80);
+
+    s_loggerCalled = false;
+    s_loggerArgs.lineNumber = 0;
+    // verify that no_originate does _not_ call the logger.
+    HresultOnLine80(winrt::hresult_error::no_originate);
+    REQUIRE(!s_loggerCalled);
+    REQUIRE(s_loggerArgs.lineNumber == 0);
+
+    // Remove global handler
+    winrt_throw_hresult_handler = nullptr;
+    s_loggerCalled = false;
+}