From d53e15af35cd4b50424c8bb046c3da1fb80aab55 Mon Sep 17 00:00:00 2001
From: Alex Malyshev <alexanderm@meta.com>
Date: Thu, 21 Mar 2024 12:26:29 -0700
Subject: [PATCH] Fix comments on Ref construction

Summary:
There's no public way to construct a Ref from a raw pointer value.  Users have
to use Ref<T>::create() or Ref<T>::steal().

Added some extra comments to BorrowedRef construction as well.

Reviewed By: swtaarrs

Differential Revision: D55070132

fbshipit-source-id: e3e0d3a011e24cccf9a2e60855e3202cc1fb5fe1
---
 cinderx/Common/ref.h | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/cinderx/Common/ref.h b/cinderx/Common/ref.h
index ead58a15d0e..fdc7f7dd59c 100644
--- a/cinderx/Common/ref.h
+++ b/cinderx/Common/ref.h
@@ -67,12 +67,16 @@ class BorrowedRef : public RefBase<T> {
     ptr_ = obj;
   }
 
+  // Allow conversion from PyObject* to any BorrowedRef<V>.
+  //
+  // PyObject doesn't have "subclasses" in the same way that C++ does, so we
+  // can't do a std::is_base_of_v<PyObject, X> check here.
   template <
       typename X = T,
       typename = std::enable_if_t<!std::is_same_v<X, PyObject>>>
   BorrowedRef(PyObject* ptr) : BorrowedRef(reinterpret_cast<X*>(ptr)) {}
 
-  // Allow conversion from any BorrowedRef to BorrowedRef<PyObject>
+  // Allow conversion from any BorrowedRef<V> to BorrowedRef<PyObject>.
   template <
       typename V,
       typename X = T,
@@ -118,7 +122,7 @@ struct std::hash<BorrowedRef<T>> {
  * One common use case is to use a Ref to create a new reference from a
  * borrowed reference that was returned from a call to the runtime, e.g.
  *
- *  Ref<> new_ref(PyDict_GetItemString(d, "key"));
+ *   auto new_ref = Ref<>::create(PyDict_GetItemString(d, "key"));
  *
  * In many cases we want to use a Ref to manage a new reference that is
  * returned as a raw PyObject* from the runtime. To do so, we steal the
@@ -126,6 +130,11 @@ struct std::hash<BorrowedRef<T>> {
  *
  *   auto stolen_ref = Ref<>::steal(PyLong_FromLong(100));
  *
+ * Note that a Ref cannot be constructed directly with a pointer value, as it's
+ * not evident whether this would be creating a new reference or stealing one:
+ *
+ *   Ref<> ref{functionThatReturnsPyObjectStar()}; // Will not compile!
+ *
  * Refs should also be used to indicate the ownership semantics of functions
  * w.r.t their arguments. Arguments that will be stolen should be Refs, whereas
  * arguments that will be borrowed should either be a BorrowedRef or a