Bug 1769518 - Support Rooted<Result<V,E>> as long as V and E have GCPolicy<> defined for them. (Use IgnoreGCPolicy for whichever of them does not need tracing.) r=emilio,nbp,jonco

Differential Revision: https://phabricator.services.mozilla.com/D146468

js/public/GCPolicyAPI.h

@@ -55,7 +55,9 @@
 namespace JS {
 
 // Defines a policy for container types with non-GC, i.e. C storage. This
-// policy dispatches to the underlying struct for GC interactions.
+// policy dispatches to the underlying struct for GC interactions. Note that
+// currently a type can define only the subset of the methods (trace and/or
+// traceWeak) if it is never used in a context that requires the other.
 template <typename T>
 struct StructGCPolicy {
   static_assert(!std::is_pointer_v<T>,
@@ -63,8 +65,6 @@ struct StructGCPolicy {
 
   static void trace(JSTracer* trc, T* tp, const char* name) { tp->trace(trc); }
 
-  static void sweep(T* tp) { return tp->sweep(); }
-
   static bool traceWeak(JSTracer* trc, T* tp) { return tp->traceWeak(trc); }
 
   static bool isValid(const T& tp) { return true; }
@@ -165,7 +165,7 @@ template <>
 struct GCPolicy<mozilla::Nothing> : public IgnoreGCPolicy<mozilla::Nothing> {};
 
 // GCPolicy<Maybe<T>> forwards tracing/sweeping to GCPolicy<T*> if
-// when the Maybe<T> is full.
+// the Maybe<T> is filled and T* can be traced via GCPolicy<T*>.
 template <typename T>
 struct GCPolicy<mozilla::Maybe<T>> {
   static void trace(JSTracer* trc, mozilla::Maybe<T>* tp, const char* name) {
@@ -190,6 +190,29 @@ struct GCPolicy<mozilla::Maybe<T>> {
 template <>
 struct GCPolicy<JS::Realm*>;  // see Realm.h
 
+template <>
+struct GCPolicy<mozilla::Ok> : public IgnoreGCPolicy<mozilla::Ok> {};
+
+template <typename V, typename E>
+struct GCPolicy<mozilla::Result<V, E>> {
+  static void trace(JSTracer* trc, mozilla::Result<V, E>* tp,
+                    const char* name) {
+    if (tp->isOk()) {
+      V tmp = tp->unwrap();
+      JS::GCPolicy<V>::trace(trc, &tmp, "Result value");
+      tp->updateAfterTracing(std::move(tmp));
+    }
+
+    if (tp->isErr()) {
+      E tmp = tp->unwrapErr();
+      JS::GCPolicy<E>::trace(trc, &tmp, "Result error");
+      tp->updateErrorAfterTracing(std::move(tmp));
+    }
+  }
+
+  static bool isValid(const mozilla::Result<V, E>& t) { return true; }
+};
+
 }  // namespace JS
 
 #endif  // GCPolicyAPI_h

js/src/jsapi-tests/testGCExactRooting.cpp

@@ -6,6 +6,8 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/Maybe.h"
+#include "mozilla/Result.h"
+#include "mozilla/ResultVariant.h"
 
 #include "ds/TraceableFifo.h"
 #include "gc/Policy.h"
@@ -587,3 +589,326 @@ bool CheckMutableOperations(T maybe) {
 }
 
 END_TEST(testRootedMaybeValue)
+
+struct TestErr {};
+struct OtherTestErr {};
+
+struct SimpleTraceable {
+  // I'm using plain objects rather than Heap<T> because Heap<T> would get
+  // traced via the store buffer. Heap<T> would be a more realistic example,
+  // but would require compaction to test for tracing.
+  JSObject* obj;
+  JS::Value val;
+
+  void trace(JSTracer* trc) {
+    TraceRoot(trc, &obj, "obj");
+    TraceRoot(trc, &val, "val");
+  }
+};
+
+namespace JS {
+template <>
+struct GCPolicy<TestErr> : public IgnoreGCPolicy<TestErr> {};
+}  // namespace JS
+
+BEGIN_TEST_WITH_ATTRIBUTES(testGCRootedResult, JS_EXPECT_HAZARDS) {
+  AutoLeaveZeal noZeal(cx);
+
+  JSObject* unrootedObj = JS_NewPlainObject(cx);
+  CHECK(js::gc::IsInsideNursery(unrootedObj));
+  Value unrootedVal = ObjectValue(*unrootedObj);
+
+  RootedObject obj(cx, unrootedObj);
+  RootedValue val(cx, unrootedVal);
+
+  Result<Value, TestErr> unrootedValerr(val);
+  Rooted<Result<Value, TestErr>> valerr(cx, val);
+
+  Result<mozilla::Ok, Value> unrootedOkval(val);
+  Rooted<Result<mozilla::Ok, Value>> okval(cx, val);
+
+  Result<mozilla::Ok, TestErr> simple{mozilla::Ok()};
+
+  Result<Value, JSObject*> unrootedValobj1(val);
+  Rooted<Result<Value, JSObject*>> valobj1(cx, val);
+  Result<Value, JSObject*> unrootedValobj2(obj);
+  Rooted<Result<Value, JSObject*>> valobj2(cx, obj);
+
+  // Test nested traceable structures.
+  Result<mozilla::Maybe<mozilla::Ok>, JSObject*> maybeobj(
+      mozilla::Some(mozilla::Ok()));
+  Rooted<Result<mozilla::Maybe<mozilla::Ok>, JSObject*>> rooted_maybeobj(
+      cx, mozilla::Some(mozilla::Ok()));
+
+  // This would fail to compile because Result<> deletes its copy constructor,
+  // which prevents updating after tracing:
+  //
+  // Rooted<Result<Result<mozilla::Ok, JS::Value>, JSObject*>>
+
+  // But this should be fine when no tracing is required.
+  Result<Result<mozilla::Ok, int>, double> dummy(3.4);
+
+  // One thing I didn't realize initially about Result<>: unwrap() takes
+  // ownership of a value. In the case of Result<Maybe>, that means the
+  // contained Maybe is reset to Nothing.
+  Result<mozilla::Maybe<int>, int> confusing(mozilla::Some(7));
+  CHECK(confusing.unwrap().isSome());
+  CHECK(!confusing.unwrap().isSome());
+
+  Result<mozilla::Maybe<JS::Value>, JSObject*> maybevalobj(
+      mozilla::Some(val.get()));
+  Rooted<Result<mozilla::Maybe<JS::Value>, JSObject*>> rooted_maybevalobj(
+      cx, mozilla::Some(val.get()));
+
+  // Custom types that haven't had GCPolicy explicitly specialized.
+  SimpleTraceable s1{obj, val};
+  Result<SimpleTraceable, TestErr> custom(s1);
+  SimpleTraceable s2{obj, val};
+  Rooted<Result<SimpleTraceable, TestErr>> rootedCustom(cx, s2);
+
+  CHECK(obj == unrootedObj);
+  CHECK(val == unrootedVal);
+  CHECK(simple.isOk());
+  CHECK(unrootedValerr.inspect() == unrootedVal);
+  CHECK(valerr.get().inspect() == val);
+  CHECK(unrootedOkval.inspectErr() == unrootedVal);
+  CHECK(okval.get().inspectErr() == val);
+  CHECK(unrootedValobj1.inspect() == unrootedVal);
+  CHECK(valobj1.get().inspect() == val);
+  CHECK(unrootedValobj2.inspectErr() == unrootedObj);
+  CHECK(valobj2.get().inspectErr() == obj);
+  CHECK(*maybevalobj.inspect() == unrootedVal);
+  CHECK(*rooted_maybevalobj.get().inspect() == val);
+  CHECK(custom.inspect().obj == unrootedObj);
+  CHECK(custom.inspect().val == unrootedVal);
+  CHECK(rootedCustom.get().inspect().obj == obj);
+  CHECK(rootedCustom.get().inspect().val == val);
+
+  JS_GC(cx);
+
+  CHECK(obj != unrootedObj);
+  CHECK(val != unrootedVal);
+  CHECK(unrootedValerr.inspect() == unrootedVal);
+  CHECK(valerr.get().inspect() == val);
+  CHECK(unrootedOkval.inspectErr() == unrootedVal);
+  CHECK(okval.get().inspectErr() == val);
+  CHECK(unrootedValobj1.inspect() == unrootedVal);
+  CHECK(valobj1.get().inspect() == val);
+  CHECK(unrootedValobj2.inspectErr() == unrootedObj);
+  CHECK(valobj2.get().inspectErr() == obj);
+  CHECK(*maybevalobj.inspect() == unrootedVal);
+  CHECK(*rooted_maybevalobj.get().inspect() == val);
+  MOZ_ASSERT(custom.inspect().obj == unrootedObj);
+  CHECK(custom.inspect().obj == unrootedObj);
+  CHECK(custom.inspect().val == unrootedVal);
+  CHECK(rootedCustom.get().inspect().obj == obj);
+  CHECK(rootedCustom.get().inspect().val == val);
+
+  mozilla::Result<OtherTestErr, mozilla::Ok> r(mozilla::Ok{});
+  (void)r;
+
+  return true;
+}
+END_TEST(testGCRootedResult)
+
+static int copies = 0;
+
+struct DontCopyMe_Variant {
+  JSObject* obj;
+  explicit DontCopyMe_Variant(JSObject* objArg) : obj(objArg) {}
+  DontCopyMe_Variant(const DontCopyMe_Variant& other) : obj(other.obj) {
+    copies++;
+  }
+  DontCopyMe_Variant(DontCopyMe_Variant&& other) : obj(std::move(other.obj)) {
+    other.obj = nullptr;
+  }
+  void trace(JSTracer* trc) { TraceRoot(trc, &obj, "obj"); }
+};
+
+enum struct TestUnusedZeroEnum : int16_t { Ok = 0, NotOk = 1 };
+
+namespace mozilla::detail {
+template <>
+struct UnusedZero<TestUnusedZeroEnum> : UnusedZeroEnum<TestUnusedZeroEnum> {};
+}  // namespace mozilla::detail
+
+namespace JS {
+template <>
+struct GCPolicy<TestUnusedZeroEnum>
+    : public IgnoreGCPolicy<TestUnusedZeroEnum> {};
+}  // namespace JS
+
+struct DontCopyMe_NullIsOk {
+  JS::Value val;
+  DontCopyMe_NullIsOk() : val(UndefinedValue()) {}
+  explicit DontCopyMe_NullIsOk(const JS::Value& valArg) : val(valArg) {}
+  DontCopyMe_NullIsOk(const DontCopyMe_NullIsOk& other) = delete;
+  DontCopyMe_NullIsOk(DontCopyMe_NullIsOk&& other)
+      : val(std::move(other.val)) {}
+  DontCopyMe_NullIsOk& operator=(DontCopyMe_NullIsOk&& other) {
+    val = std::move(other.val);
+    other.val = UndefinedValue();
+    return *this;
+  }
+  void trace(JSTracer* trc) { TraceRoot(trc, &val, "val"); }
+};
+
+struct Failed {};
+
+namespace mozilla::detail {
+template <>
+struct UnusedZero<Failed> {
+  using StorageType = uintptr_t;
+
+  static constexpr bool value = true;
+  static constexpr StorageType nullValue = 0;
+  static constexpr StorageType GetDefaultValue() { return 2; }
+
+  static constexpr void AssertValid(StorageType aValue) {}
+  static constexpr Failed Inspect(const StorageType& aValue) {
+    return Failed{};
+  }
+  static constexpr Failed Unwrap(StorageType aValue) { return Failed{}; }
+  static constexpr StorageType Store(Failed aValue) {
+    return GetDefaultValue();
+  }
+};
+}  // namespace mozilla::detail
+
+namespace JS {
+template <>
+struct GCPolicy<Failed> : public IgnoreGCPolicy<Failed> {};
+}  // namespace JS
+
+struct TriviallyCopyable_LowBitTagIsError {
+  JSObject* obj;
+  TriviallyCopyable_LowBitTagIsError() : obj(nullptr) {}
+  explicit TriviallyCopyable_LowBitTagIsError(JSObject* objArg) : obj(objArg) {}
+  TriviallyCopyable_LowBitTagIsError(
+      const TriviallyCopyable_LowBitTagIsError& other) = default;
+  void trace(JSTracer* trc) { TraceRoot(trc, &obj, "obj"); }
+};
+
+namespace mozilla::detail {
+template <>
+struct HasFreeLSB<TriviallyCopyable_LowBitTagIsError> : HasFreeLSB<JSObject*> {
+};
+}  // namespace mozilla::detail
+
+BEGIN_TEST_WITH_ATTRIBUTES(testRootedResultCtors, JS_EXPECT_HAZARDS) {
+  JSObject* unrootedObj = JS_NewPlainObject(cx);
+  CHECK(unrootedObj);
+  Rooted<JSObject*> obj(cx, unrootedObj);
+
+  using mozilla::detail::PackingStrategy;
+
+  static_assert(Result<DontCopyMe_Variant, TestErr>::Strategy ==
+                PackingStrategy::Variant);
+  Rooted<Result<DontCopyMe_Variant, TestErr>> vv(cx, DontCopyMe_Variant{obj});
+  static_assert(Result<mozilla::Ok, DontCopyMe_Variant>::Strategy ==
+                PackingStrategy::Variant);
+  Rooted<Result<mozilla::Ok, DontCopyMe_Variant>> ve(cx,
+                                                     DontCopyMe_Variant{obj});
+
+  static_assert(Result<DontCopyMe_NullIsOk, TestUnusedZeroEnum>::Strategy ==
+                PackingStrategy::NullIsOk);
+  Rooted<Result<DontCopyMe_NullIsOk, TestUnusedZeroEnum>> nv(
+      cx, DontCopyMe_NullIsOk{JS::ObjectValue(*obj)});
+
+  static_assert(Result<TriviallyCopyable_LowBitTagIsError, Failed>::Strategy ==
+                PackingStrategy::LowBitTagIsError);
+  Rooted<Result<TriviallyCopyable_LowBitTagIsError, Failed>> lv(
+      cx, TriviallyCopyable_LowBitTagIsError{obj});
+
+  CHECK(obj == unrootedObj);
+
+  CHECK(vv.get().inspect().obj == obj);
+  CHECK(ve.get().inspectErr().obj == obj);
+  CHECK(nv.get().inspect().val.toObjectOrNull() == obj);
+  CHECK(lv.get().inspect().obj == obj);
+
+  JS_GC(cx);
+  CHECK(obj != unrootedObj);
+
+  CHECK(vv.get().inspect().obj == obj);
+  CHECK(ve.get().inspectErr().obj == obj);
+  CHECK(nv.get().inspect().val.toObjectOrNull() == obj);
+  CHECK(lv.get().inspect().obj == obj);
+  CHECK(copies == 0);
+  return true;
+}
+END_TEST(testRootedResultCtors)
+
+#if defined(HAVE_64BIT_BUILD) && !defined(XP_WIN)
+
+// This depends on a pointer fitting in 48 bits, leaving space for an empty
+// struct and a bool in a packed struct. Windows doesn't seem to do this
+// packing, so we'll skip this test here. We're primarily checking whether
+// copy constructors get called, which should be cross-platform, and
+// secondarily making sure that the Rooted/tracing stuff is compiled and
+// executed properly. There are certainly more clever ways to do this that
+// would work cross-platform, but it doesn't seem worth the bother right now.
+
+struct __attribute__((packed)) DontCopyMe_PackedVariant {
+  uintptr_t obj : 48;
+  static JSObject* Unwrap(uintptr_t packed) {
+    return reinterpret_cast<JSObject*>(packed);
+  }
+  static uintptr_t Store(JSObject* obj) {
+    return reinterpret_cast<uintptr_t>(obj);
+  }
+
+  DontCopyMe_PackedVariant() : obj(0) {}
+  explicit DontCopyMe_PackedVariant(JSObject* objArg)
+      : obj(reinterpret_cast<uintptr_t>(objArg)) {}
+  DontCopyMe_PackedVariant(const DontCopyMe_PackedVariant& other)
+      : obj(other.obj) {
+    copies++;
+  }
+  DontCopyMe_PackedVariant(DontCopyMe_PackedVariant&& other) : obj(other.obj) {
+    other.obj = 0;
+  }
+  void trace(JSTracer* trc) {
+    JSObject* realObj = Unwrap(obj);
+    TraceRoot(trc, &realObj, "obj");
+    obj = Store(realObj);
+  }
+};
+
+static_assert(std::is_default_constructible_v<DontCopyMe_PackedVariant>);
+static_assert(std::is_default_constructible_v<TestErr>);
+static_assert(mozilla::detail::IsPackableVariant<DontCopyMe_PackedVariant,
+                                                 TestErr>::value);
+
+BEGIN_TEST_WITH_ATTRIBUTES(testResultPackedVariant, JS_EXPECT_HAZARDS) {
+  JSObject* unrootedObj = JS_NewPlainObject(cx);
+  CHECK(unrootedObj);
+  Rooted<JSObject*> obj(cx, unrootedObj);
+
+  using mozilla::detail::PackingStrategy;
+
+  static_assert(Result<DontCopyMe_PackedVariant, TestErr>::Strategy ==
+                PackingStrategy::PackedVariant);
+  Rooted<Result<DontCopyMe_PackedVariant, TestErr>> pv(
+      cx, DontCopyMe_PackedVariant{obj});
+  static_assert(Result<mozilla::Ok, DontCopyMe_PackedVariant>::Strategy ==
+                PackingStrategy::PackedVariant);
+  Rooted<Result<mozilla::Ok, DontCopyMe_PackedVariant>> pe(
+      cx, DontCopyMe_PackedVariant{obj});
+
+  CHECK(obj == unrootedObj);
+
+  CHECK(DontCopyMe_PackedVariant::Unwrap(pv.get().inspect().obj) == obj);
+  CHECK(DontCopyMe_PackedVariant::Unwrap(pe.get().inspectErr().obj) == obj);
+
+  JS_GC(cx);
+  CHECK(obj != unrootedObj);
+
+  CHECK(DontCopyMe_PackedVariant::Unwrap(pv.get().inspect().obj) == obj);
+  CHECK(DontCopyMe_PackedVariant::Unwrap(pe.get().inspectErr().obj) == obj);
+
+  return true;
+}
+END_TEST(testResultPackedVariant)
+
+#endif  // HAVE_64BIT_BUILD

mfbt/Result.h

@@ -138,11 +138,21 @@ class ResultImplementationNullIsOkBase {
 
   constexpr const V& inspect() const { return *mValue.first().addr(); }
   constexpr V unwrap() { return std::move(*mValue.first().addr()); }
+  constexpr void updateAfterTracing(V&& aValue) {
+    MOZ_ASSERT(isOk());
+    if (!std::is_empty_v<V>) {
+      mValue.first().addr()->~V();
+      new (mValue.first().addr()) V(std::move(aValue));
+    }
+  }
 
   constexpr decltype(auto) inspectErr() const {
     return UnusedZero<E>::Inspect(mValue.second());
   }
   constexpr E unwrapErr() { return UnusedZero<E>::Unwrap(mValue.second()); }
+  constexpr void updateErrorAfterTracing(E&& aErrorValue) {
+    mValue.second() = UnusedZero<E>::Store(std::move(aErrorValue));
+  }
 };
 
 template <typename V, typename E,
@@ -184,6 +194,7 @@ template <typename V, typename E>
 class ResultImplementation<V, E, PackingStrategy::NullIsOk>
     : public ResultImplementationNullIsOk<V, E> {
  public:
+  static constexpr PackingStrategy Strategy = PackingStrategy::NullIsOk;
   using ResultImplementationNullIsOk<V, E>::ResultImplementationNullIsOk;
 };
 
@@ -222,6 +233,8 @@ class ResultImplementation<V, E, PackingStrategy::LowBitTagIsError> {
 #endif
 
  public:
+  static constexpr PackingStrategy Strategy = PackingStrategy::LowBitTagIsError;
+
   explicit constexpr ResultImplementation(V aValue) : mBits(0) {
     if constexpr (!std::is_empty_v<V>) {
       std::memcpy(&mBits, &aValue, sizeof(V));
@@ -256,17 +269,32 @@ class ResultImplementation<V, E, PackingStrategy::LowBitTagIsError> {
     return res;
   }
   constexpr E unwrapErr() { return inspectErr(); }
+
+  constexpr void updateAfterTracing(V&& aValue) {
+    this->~ResultImplementation();
+    new (this) ResultImplementation(std::move(aValue));
+  }
+  constexpr void updateErrorAfterTracing(E&& aErrorValue) {
+    this->~ResultImplementation();
+    new (this) ResultImplementation(std::move(aErrorValue));
+  }
 };
 
 // Return true if any of the struct can fit in a word.
 template <typename V, typename E>
 struct IsPackableVariant {
   struct VEbool {
+    explicit constexpr VEbool(V&& aValue) : v(std::move(aValue)), ok(true) {}
+    explicit constexpr VEbool(E&& aErrorValue)
+        : e(std::move(aErrorValue)), ok(false) {}
     V v;
     E e;
     bool ok;
   };
   struct EVbool {
+    explicit constexpr EVbool(V&& aValue) : v(std::move(aValue)), ok(true) {}
+    explicit constexpr EVbool(E&& aErrorValue)
+        : e(std::move(aErrorValue)), ok(false) {}
     E e;
     V v;
     bool ok;
@@ -288,14 +316,11 @@ class ResultImplementation<V, E, PackingStrategy::PackedVariant> {
   Impl data;
 
  public:
-  explicit constexpr ResultImplementation(V aValue) {
+  static constexpr PackingStrategy Strategy = PackingStrategy::PackedVariant;
-    data.v = std::move(aValue);
+
-    data.ok = true;
+  explicit constexpr ResultImplementation(V aValue) : data(std::move(aValue)) {}
-  }
+  explicit constexpr ResultImplementation(E aErrorValue)
-  explicit constexpr ResultImplementation(E aErrorValue) {
+      : data(std::move(aErrorValue)) {}
-    data.e = std::move(aErrorValue);
-    data.ok = false;
-  }
 
   constexpr bool isOk() const { return data.ok; }
 
@@ -304,6 +329,17 @@ class ResultImplementation<V, E, PackingStrategy::PackedVariant> {
 
   constexpr const E& inspectErr() const { return data.e; }
   constexpr E unwrapErr() { return std::move(data.e); }
+
+  constexpr void updateAfterTracing(V&& aValue) {
+    MOZ_ASSERT(data.ok);
+    this->~ResultImplementation();
+    new (this) ResultImplementation(std::move(aValue));
+  }
+  constexpr void updateErrorAfterTracing(E&& aErrorValue) {
+    MOZ_ASSERT(!data.ok);
+    this->~ResultImplementation();
+    new (this) ResultImplementation(std::move(aErrorValue));
+  }
 };
 
 // To use nullptr as a special value, we need the counter part to exclude zero
@@ -456,11 +492,12 @@ class [[nodiscard]] Result final {
   // You need to include "ResultVariant.h"!
 
  public:
+  static constexpr detail::PackingStrategy Strategy = Impl::Strategy;
   using ok_type = V;
   using err_type = E;
 
   /** Create a success result. */
-  MOZ_IMPLICIT constexpr Result(V&& aValue) : mImpl(std::forward<V>(aValue)) {
+  MOZ_IMPLICIT constexpr Result(V&& aValue) : mImpl(std::move(aValue)) {
     MOZ_ASSERT(isOk());
   }
 
@@ -477,7 +514,10 @@ class [[nodiscard]] Result final {
   }
 
   /** Create an error result. */
-  explicit constexpr Result(E aErrorValue) : mImpl(std::move(aErrorValue)) {
+  explicit constexpr Result(const E& aErrorValue) : mImpl(aErrorValue) {
+    MOZ_ASSERT(isErr());
+  }
+  explicit constexpr Result(E&& aErrorValue) : mImpl(std::move(aErrorValue)) {
     MOZ_ASSERT(isErr());
   }
 
@@ -544,6 +584,18 @@ class [[nodiscard]] Result final {
     return mImpl.unwrapErr();
   }
 
+  /** Used only for GC tracing. If used in Rooted<Result<...>>, V must have a
+   * GCPolicy for tracing it. */
+  constexpr void updateAfterTracing(V&& aValue) {
+    mImpl.updateAfterTracing(std::move(aValue));
+  }
+
+  /** Used only for GC tracing. If used in Rooted<Result<...>>, E must have a
+   * GCPolicy for tracing it. */
+  constexpr void updateErrorAfterTracing(E&& aErrorValue) {
+    mImpl.updateErrorAfterTracing(std::move(aErrorValue));
+  }
+
   /** See the success value from this Result, which must be a success result. */
   constexpr decltype(auto) inspect() const {
     static_assert(!std::is_reference_v<

mfbt/ResultVariant.h

@@ -20,13 +20,14 @@ class ResultImplementation<V, E, PackingStrategy::Variant> {
   mozilla::Variant<V, E> mStorage;
 
  public:
+  static constexpr PackingStrategy Strategy = PackingStrategy::Variant;
+
   ResultImplementation(ResultImplementation&&) = default;
   ResultImplementation(const ResultImplementation&) = delete;
   ResultImplementation& operator=(const ResultImplementation&) = delete;
   ResultImplementation& operator=(ResultImplementation&&) = default;
 
-  explicit ResultImplementation(V&& aValue)
+  explicit ResultImplementation(V&& aValue) : mStorage(std::move(aValue)) {}
-      : mStorage(std::forward<V>(aValue)) {}
   explicit ResultImplementation(const V& aValue) : mStorage(aValue) {}
   template <typename... Args>
   explicit ResultImplementation(std::in_place_t, Args&&... aArgs)
@@ -34,7 +35,7 @@ class ResultImplementation<V, E, PackingStrategy::Variant> {
 
   explicit ResultImplementation(const E& aErrorValue) : mStorage(aErrorValue) {}
   explicit ResultImplementation(E&& aErrorValue)
-      : mStorage(std::forward<E>(aErrorValue)) {}
+      : mStorage(std::move(aErrorValue)) {}
 
   bool isOk() const { return mStorage.template is<V>(); }
 
@@ -46,6 +47,13 @@ class ResultImplementation<V, E, PackingStrategy::Variant> {
 
   E unwrapErr() { return std::move(mStorage.template as<E>()); }
   const E& inspectErr() const { return mStorage.template as<E>(); }
+
+  void updateAfterTracing(V&& aValue) {
+    mStorage.template emplace<V>(std::move(aValue));
+  }
+  void updateErrorAfterTracing(E&& aErrorValue) {
+    mStorage.template emplace<E>(std::move(aErrorValue));
+  }
 };
 
 }  // namespace mozilla::detail