commit a2ea597e7aca5ca96a36353a65df04d18a72b70c
author Tom Sepez <tsepez@chromium.org> Fri Sep 02 16:30:59 2022 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Sep 02 16:30:59 2022 +0000
tree e89e3b2e24e47d3f1f077ef7ae6ec06c7a86210f
parent 779b542c6ae2fe9b70475a15617ef1deac5a6b9b

Avoid ambiguity in RetainPtr<> constructors

When two different RetainPtr<> types appear in an expression, and
one is convertible to the other but not vice-versa, SFINAE is not
of any help because the compiler first has to pick a direction in
which to convert (i.e. it won't try both and reject the one that
fails).

-- code taken from base/memory/scoped_refptr.h
-- add a test case that fails to compile without the patch.

Change-Id: I7fea264643bf4b9343336cf68253dcac7199a71b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97351
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

core/fxcrt/retain_ptr.h

diff --git a/core/fxcrt/retain_ptr.h b/core/fxcrt/retain_ptr.h
index b8bc6a0..d227257 100644
--- a/core/fxcrt/retain_ptr.h
+++ b/core/fxcrt/retain_ptr.h
@@ -9,6 +9,7 @@
 
 #include <functional>
 #include <memory>
+#include <type_traits>
 #include <utility>
 
 #include "core/fxcrt/unowned_ptr.h"
@@ -46,11 +47,15 @@
   RetainPtr(std::nullptr_t ptr) {}
 
   // Copy conversion constructor.
-  template <class U>
+  template <class U,
+            typename = typename std::enable_if<
+                std::is_convertible<U*, T*>::value>::type>
   RetainPtr(const RetainPtr<U>& that) : RetainPtr(that.Get()) {}
 
   // Move-conversion constructor.
-  template <class U>
+  template <class U,
+            typename = typename std::enable_if<
+                std::is_convertible<U*, T*>::value>::type>
   RetainPtr(RetainPtr<U>&& that) noexcept {
     Unleak(that.Leak());
   }

core/fxcrt/retain_ptr_unittest.cpp

diff --git a/core/fxcrt/retain_ptr_unittest.cpp b/core/fxcrt/retain_ptr_unittest.cpp
index aae889c..84063fb 100644
--- a/core/fxcrt/retain_ptr_unittest.cpp
+++ b/core/fxcrt/retain_ptr_unittest.cpp
@@ -97,6 +97,15 @@
   EXPECT_EQ(1, obj.release_count());
 }
 
+TEST(RetainPtr, AmbiguousExpression) {
+  class A : public Retainable {};
+  class B : public A {};
+
+  // Test passes if it compiles without error.
+  RetainPtr<A> var = (0) ? pdfium::MakeRetain<A>() : pdfium::MakeRetain<B>();
+  EXPECT_TRUE(var);
+}
+
 TEST(RetainPtr, ResetNull) {
   PseudoRetainable obj;
   {