Bug 1538779 - Make `Rooted<MyContainer> c(cx)` the equivalent of `Rooted<MyContainer> c(cx, MyContainer(cx))` if possible r=jonco
☠☠ backed out by aec4b19308a9 ☠ ☠
authorSteve Fink <sfink@mozilla.com>
Mon, 29 Apr 2019 17:38:14 +0000
changeset 471783 f2a3fb166dfba14d854a49668aeca6cce6bb3b55
parent 471782 9b357ff266758e6826c3addf13916697b8f131be
child 471784 06792d1b08fd016bfeec0976687989ffe44bae75
push id35934
push usershindli@mozilla.com
push dateMon, 29 Apr 2019 21:53:38 +0000
treeherdermozilla-central@f6766ba4ac77 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjonco
bugs1538779
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1538779 - Make `Rooted<MyContainer> c(cx)` the equivalent of `Rooted<MyContainer> c(cx, MyContainer(cx))` if possible r=jonco Differential Revision: https://phabricator.services.mozilla.com/D26797
js/public/GCPolicyAPI.h
js/public/RootingAPI.h
js/src/jsapi-tests/testGCExactRooting.cpp
--- a/js/public/GCPolicyAPI.h
+++ b/js/public/GCPolicyAPI.h
@@ -23,17 +23,17 @@
 //         GCHashSet use this method to decide when to remove an entry: if this
 //         function returns true on a key/value/member/etc, its entry is dropped
 //         from the container. Specializing this method is the standard way to
 //         get custom weak behavior from a container type.
 //
 //   static bool isValid(const T& t)
 //       - Return false only if |t| is corrupt in some way. The built-in GC
 //         types do some memory layout checks. For debugging only; it is ok
-//         to always return true.
+//         to always return true or even to omit this member entirely.
 //
 // The default GCPolicy<T> assumes that T has a default constructor and |trace|
 // and |needsSweep| methods, and forwards to them. GCPolicy has appropriate
 // specializations for pointers to GC things and pointer-like types like
 // JS::Heap<T> and mozilla::UniquePtr<T>.
 //
 // There are some stock structs your specializations can inherit from.
 // IgnoreGCPolicy<T> does nothing. StructGCPolicy<T> forwards the methods to the
@@ -199,11 +199,38 @@ struct GCPolicy<mozilla::Maybe<T>> {
     }
     return true;
   }
 };
 
 template <>
 struct GCPolicy<JS::Realm*>;  // see Realm.h
 
+namespace detail {
+
+// Dummy types to make it easier to understand template overload preference
+// ordering.
+struct FallbackOverload {};
+struct PreferredOverload : FallbackOverload {};
+using OverloadSelector = PreferredOverload;
+
+// A version of GCPolicy<T> that is guaranteed to have an isValid member.
+template <typename T>
+struct GCPolicyWithIsValid : GCPolicy<T> {
+  template <typename U = T, typename = decltype(GCPolicy<U>::isValid)>
+  static bool isValidPicker(const T& t, detail::PreferredOverload) {
+    return GCPolicy<T>::isValid(t);
+  }
+
+  static bool isValidPicker(const T& t, detail::FallbackOverload) {
+    return true;
+  }
+
+  static bool isValid(const T& t) {
+    return isValidPicker(t, detail::OverloadSelector());
+  }
+};
+
+}  // namespace detail
+
 }  // namespace JS
 
 #endif  // GCPolicyAPI_h
--- a/js/public/RootingAPI.h
+++ b/js/public/RootingAPI.h
@@ -634,21 +634,21 @@ class MOZ_STACK_CLASS MutableHandle
 
  private:
   // Disallow nullptr for overloading purposes.
   MutableHandle(decltype(nullptr)) = delete;
 
  public:
   void set(const T& v) {
     *ptr = v;
-    MOZ_ASSERT(GCPolicy<T>::isValid(*ptr));
+    MOZ_ASSERT(detail::GCPolicyWithIsValid<T>::isValid(*ptr));
   }
   void set(T&& v) {
     *ptr = std::move(v);
-    MOZ_ASSERT(GCPolicy<T>::isValid(*ptr));
+    MOZ_ASSERT(detail::GCPolicyWithIsValid<T>::isValid(*ptr));
   }
 
   /*
    * This may be called only if the location of the T is guaranteed
    * to be marked (for some reason other than being a Rooted),
    * e.g., if it is guaranteed to be reachable from an implicit root.
    *
    * Create a MutableHandle from a raw location of a T.
@@ -1007,49 +1007,72 @@ class MOZ_RAII Rooted : public js::Roote
 
   inline RootedListHeads& rootLists(RootingContext* cx) {
     return cx->stackRoots_;
   }
   inline RootedListHeads& rootLists(JSContext* cx) {
     return rootLists(RootingContext::get(cx));
   }
 
+  // Define either one or two Rooted(cx) constructors: the fallback one, which
+  // constructs a Rooted holding a SafelyInitialized<T>, and a convenience one
+  // for types that can be constructed with a cx, which will give a Rooted
+  // holding a T(cx).
+
+  // Dummy type to distinguish these constructors from Rooted(cx, initial)
+  struct CtorDispatcher {};
+
+  // Normal case: construct an empty Rooted holding a safely initialized but
+  // empty T.
+  template <typename RootingContext>
+  Rooted(const RootingContext& cx, CtorDispatcher, detail::FallbackOverload)
+    : Rooted(cx, SafelyInitialized<T>()) {}
+
+  // If T can be constructed with a cx, then define another constructor for it
+  // that will be preferred.
+  template <typename RootingContext,
+            typename = decltype(T(std::declval<RootingContext>()))>
+  Rooted(const RootingContext& cx, CtorDispatcher, detail::PreferredOverload)
+      : Rooted(cx, T(cx)) {}
+
  public:
   using ElementType = T;
 
+  // Construct an empty Rooted. Delegates to an internal constructor that
+  // chooses a specific meaning of "empty" depending on whether T can be
+  // constructed with a cx.
   template <typename RootingContext>
-  explicit Rooted(const RootingContext& cx) : ptr(SafelyInitialized<T>()) {
-    registerWithRootLists(rootLists(cx));
-  }
+  explicit Rooted(const RootingContext& cx)
+      : Rooted(cx, CtorDispatcher(), detail::OverloadSelector()) {}
 
   template <typename RootingContext, typename S>
   Rooted(const RootingContext& cx, S&& initial)
       : ptr(std::forward<S>(initial)) {
-    MOZ_ASSERT(GCPolicy<T>::isValid(ptr));
+    MOZ_ASSERT(detail::GCPolicyWithIsValid<T>::isValid(ptr));
     registerWithRootLists(rootLists(cx));
   }
 
   ~Rooted() {
     MOZ_ASSERT(*stack == reinterpret_cast<Rooted<void*>*>(this));
     *stack = prev;
   }
 
   Rooted<T>* previous() { return reinterpret_cast<Rooted<T>*>(prev); }
 
   /*
    * This method is public for Rooted so that Codegen.py can use a Rooted
    * interchangeably with a MutableHandleValue.
    */
   void set(const T& value) {
     ptr = value;
-    MOZ_ASSERT(GCPolicy<T>::isValid(ptr));
+    MOZ_ASSERT(detail::GCPolicyWithIsValid<T>::isValid(ptr));
   }
   void set(T&& value) {
     ptr = std::move(value);
-    MOZ_ASSERT(GCPolicy<T>::isValid(ptr));
+    MOZ_ASSERT(detail::GCPolicyWithIsValid<T>::isValid(ptr));
   }
 
   DECLARE_POINTER_CONSTREF_OPS(T);
   DECLARE_POINTER_ASSIGN_OPS(Rooted, T);
   DECLARE_NONPOINTER_ACCESSOR_METHODS(ptr);
   DECLARE_NONPOINTER_MUTABLE_ACCESSOR_METHODS(ptr);
 
  private:
--- a/js/src/jsapi-tests/testGCExactRooting.cpp
+++ b/js/src/jsapi-tests/testGCExactRooting.cpp
@@ -244,17 +244,17 @@ BEGIN_TEST(testGCHandleHashMap) {
 
   return true;
 }
 END_TEST(testGCHandleHashMap)
 
 using ShapeVec = GCVector<Shape*>;
 
 BEGIN_TEST(testGCRootedVector) {
-  JS::Rooted<ShapeVec> shapes(cx, ShapeVec(cx));
+  JS::Rooted<ShapeVec> shapes(cx);
 
   for (size_t i = 0; i < 10; ++i) {
     RootedObject obj(cx, JS_NewObject(cx, nullptr));
     RootedValue val(cx, UndefinedValue());
     // Construct a unique property name to ensure that the object creates a
     // new shape.
     char buffer[2];
     buffer[0] = 'a' + i;