From f2863b67ef7a1bb89e2befb6228094eeeb043030 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Mon, 25 Oct 2021 21:18:56 +0000 Subject: [PATCH] Bug 1736737 - Move SafelyInitialized into a struct to allow partial specialization. r=jonco Differential Revision: https://phabricator.services.mozilla.com/D129345 --- js/public/Debug.h | 2 +- js/public/RootingAPI.h | 69 ++++++++++++++++++--------------- js/src/gc/Barrier.h | 46 +++++++++++----------- js/src/gc/MaybeRooted.h | 3 +- js/src/gc/NurseryAwareHashMap.h | 5 ++- js/src/vm/Interpreter.cpp | 4 +- 6 files changed, 69 insertions(+), 60 deletions(-) diff --git a/js/public/Debug.h b/js/public/Debug.h index 39e7b8af48ed..ed025d5ca98b 100644 --- a/js/public/Debug.h +++ b/js/public/Debug.h @@ -160,7 +160,7 @@ class Builder { PersistentRooted value; BuiltThing(JSContext* cx, Builder& owner_, - T value_ = SafelyInitialized()) + T value_ = SafelyInitialized::create()) : owner(owner_), value(cx, value_) { owner.assertBuilt(value_); } diff --git a/js/public/RootingAPI.h b/js/public/RootingAPI.h index 167d8223978a..2f9aca637e4d 100644 --- a/js/public/RootingAPI.h +++ b/js/public/RootingAPI.h @@ -215,37 +215,41 @@ JS_PUBLIC_API void HeapScriptWriteBarriers(JSScript** objp, JSScript* prev, JSScript* next); /** - * Create a safely-initialized |T|, suitable for use as a default value in - * situations requiring a safe but arbitrary |T| value. + * SafelyInitialized::create() creates a safely-initialized |T|, suitable for + * use as a default value in situations requiring a safe but arbitrary |T| + * value. Implemented as a static method of a struct to allow partial + * specialization for subclasses via the Enable template parameter. */ -template -inline T SafelyInitialized() { - // This function wants to presume that |T()| -- which value-initializes a - // |T| per C++11 [expr.type.conv]p2 -- will produce a safely-initialized, - // safely-usable T that it can return. +template +struct SafelyInitialized { + static T create() { + // This function wants to presume that |T()| -- which value-initializes a + // |T| per C++11 [expr.type.conv]p2 -- will produce a safely-initialized, + // safely-usable T that it can return. #if defined(XP_WIN) || defined(XP_MACOSX) || \ (defined(XP_UNIX) && !defined(__clang__)) - // That presumption holds for pointers, where value initialization produces - // a null pointer. - constexpr bool IsPointer = std::is_pointer_v; + // That presumption holds for pointers, where value initialization produces + // a null pointer. + constexpr bool IsPointer = std::is_pointer_v; - // For classes and unions we *assume* that if |T|'s default constructor is - // non-trivial it'll initialize correctly. (This is unideal, but C++ - // doesn't offer a type trait indicating whether a class's constructor is - // user-defined, which better approximates our desired semantics.) - constexpr bool IsNonTriviallyDefaultConstructibleClassOrUnion = - (std::is_class_v || - std::is_union_v)&&!std::is_trivially_default_constructible_v; + // For classes and unions we *assume* that if |T|'s default constructor is + // non-trivial it'll initialize correctly. (This is unideal, but C++ + // doesn't offer a type trait indicating whether a class's constructor is + // user-defined, which better approximates our desired semantics.) + constexpr bool IsNonTriviallyDefaultConstructibleClassOrUnion = + (std::is_class_v || + std::is_union_v)&&!std::is_trivially_default_constructible_v; - static_assert(IsPointer || IsNonTriviallyDefaultConstructibleClassOrUnion, - "T() must evaluate to a safely-initialized T"); + static_assert(IsPointer || IsNonTriviallyDefaultConstructibleClassOrUnion, + "T() must evaluate to a safely-initialized T"); #endif - return T(); -} + return T(); + } +}; #ifdef JS_DEBUG /** @@ -301,13 +305,13 @@ class MOZ_NON_MEMMOVABLE Heap : public js::HeapOperations> { public: using ElementType = T; - Heap() : ptr(SafelyInitialized()) { + Heap() : ptr(SafelyInitialized::create()) { // No barriers are required for initialization to the default value. static_assert(sizeof(T) == sizeof(Heap), "Heap must be binary compatible with T."); } explicit Heap(const T& p) : ptr(p) { - postWriteBarrier(SafelyInitialized(), ptr); + postWriteBarrier(SafelyInitialized::create(), ptr); } /* @@ -317,19 +321,19 @@ class MOZ_NON_MEMMOVABLE Heap : public js::HeapOperations> { * though they are equivalent. */ explicit Heap(const Heap& other) : ptr(other.getWithoutExpose()) { - postWriteBarrier(SafelyInitialized(), ptr); + postWriteBarrier(SafelyInitialized::create(), ptr); } Heap(Heap&& other) : ptr(other.getWithoutExpose()) { - postWriteBarrier(SafelyInitialized(), ptr); + postWriteBarrier(SafelyInitialized::create(), ptr); } Heap& operator=(Heap&& other) { set(other.getWithoutExpose()); - other.set(SafelyInitialized()); + other.set(SafelyInitialized::create()); return *this; } - ~Heap() { postWriteBarrier(ptr, SafelyInitialized()); } + ~Heap() { postWriteBarrier(ptr, SafelyInitialized::create()); } DECLARE_POINTER_CONSTREF_OPS(T); DECLARE_POINTER_ASSIGN_OPS(Heap, T); @@ -1146,7 +1150,8 @@ class MOZ_RAII Rooted : public detail::RootedTraits::StackBase, template , RootingContext>> - explicit Rooted(const RootingContext& cx) : ptr(SafelyInitialized()) { + explicit Rooted(const RootingContext& cx) + : ptr(SafelyInitialized::create()) { registerWithRootLists(rootLists(cx)); } @@ -1394,13 +1399,13 @@ class PersistentRooted : public detail::RootedTraits::PersistentBase, public: using ElementType = T; - PersistentRooted() : ptr(SafelyInitialized()) {} + PersistentRooted() : ptr(SafelyInitialized::create()) {} template < typename RootHolder, typename = std::enable_if_t, RootHolder>> explicit PersistentRooted(const RootHolder& cx) - : ptr(SafelyInitialized()) { + : ptr(SafelyInitialized::create()) { registerWithRootLists(cx); } @@ -1433,7 +1438,7 @@ class PersistentRooted : public detail::RootedTraits::PersistentBase, bool initialized() const { return this->isInList(); } - void init(RootingContext* cx) { init(cx, SafelyInitialized()); } + void init(RootingContext* cx) { init(cx, SafelyInitialized::create()); } void init(JSContext* cx) { init(RootingContext::get(cx)); } template @@ -1449,7 +1454,7 @@ class PersistentRooted : public detail::RootedTraits::PersistentBase, void reset() { if (initialized()) { - set(SafelyInitialized()); + set(SafelyInitialized::create()); this->remove(); } } diff --git a/js/src/gc/Barrier.h b/js/src/gc/Barrier.h index 0a842ad7393f..a3bb5267c7da 100644 --- a/js/src/gc/Barrier.h +++ b/js/src/gc/Barrier.h @@ -501,7 +501,7 @@ class WriteBarriered : public BarrieredBase, template class PreBarriered : public WriteBarriered { public: - PreBarriered() : WriteBarriered(JS::SafelyInitialized()) {} + PreBarriered() : WriteBarriered(JS::SafelyInitialized::create()) {} /* * Allow implicit construction for use in generic contexts, such as * DebuggerWeakMap::markKeys. @@ -518,7 +518,7 @@ class PreBarriered : public WriteBarriered { void init(const T& v) { this->value = v; } /* Use to set the pointer to nullptr. */ - void clear() { set(JS::SafelyInitialized()); } + void clear() { set(JS::SafelyInitialized::create()); } DECLARE_POINTER_ASSIGN_AND_MOVE_OPS(PreBarriered, T); @@ -535,7 +535,7 @@ class PreBarriered : public WriteBarriered { T release() { T tmp = this->value; - this->value = JS::SafelyInitialized(); + this->value = JS::SafelyInitialized::create(); return tmp; } }; @@ -571,14 +571,14 @@ namespace js { template class GCPtr : public WriteBarriered { public: - GCPtr() : WriteBarriered(JS::SafelyInitialized()) {} + GCPtr() : WriteBarriered(JS::SafelyInitialized::create()) {} explicit GCPtr(const T& v) : WriteBarriered(v) { - this->post(JS::SafelyInitialized(), v); + this->post(JS::SafelyInitialized::create(), v); } explicit GCPtr(const GCPtr& v) : WriteBarriered(v) { - this->post(JS::SafelyInitialized(), v); + this->post(JS::SafelyInitialized::create(), v); } #ifdef DEBUG @@ -597,7 +597,7 @@ class GCPtr : public WriteBarriered { void init(const T& v) { AssertTargetIsNotGray(v); this->value = v; - this->post(JS::SafelyInitialized(), v); + this->post(JS::SafelyInitialized::create(), v); } DECLARE_POINTER_ASSIGN_OPS(GCPtr, T); @@ -667,31 +667,31 @@ namespace js { template class HeapPtr : public WriteBarriered { public: - HeapPtr() : WriteBarriered(JS::SafelyInitialized()) {} + HeapPtr() : WriteBarriered(JS::SafelyInitialized::create()) {} // Implicitly adding barriers is a reasonable default. MOZ_IMPLICIT HeapPtr(const T& v) : WriteBarriered(v) { - this->post(JS::SafelyInitialized(), this->value); + this->post(JS::SafelyInitialized::create(), this->value); } MOZ_IMPLICIT HeapPtr(const HeapPtr& other) : WriteBarriered(other) { - this->post(JS::SafelyInitialized(), this->value); + this->post(JS::SafelyInitialized::create(), this->value); } HeapPtr(HeapPtr&& other) : WriteBarriered(other.release()) { - this->post(JS::SafelyInitialized(), this->value); + this->post(JS::SafelyInitialized::create(), this->value); } ~HeapPtr() { this->pre(); - this->post(this->value, JS::SafelyInitialized()); + this->post(this->value, JS::SafelyInitialized::create()); } void init(const T& v) { - MOZ_ASSERT(this->value == JS::SafelyInitialized()); + MOZ_ASSERT(this->value == JS::SafelyInitialized::create()); AssertTargetIsNotGray(v); this->value = v; - this->post(JS::SafelyInitialized(), this->value); + this->post(JS::SafelyInitialized::create(), this->value); } DECLARE_POINTER_ASSIGN_AND_MOVE_OPS(HeapPtr, T); @@ -720,7 +720,7 @@ class HeapPtr : public WriteBarriered { T release() { T tmp = this->value; - postBarrieredSet(JS::SafelyInitialized()); + postBarrieredSet(JS::SafelyInitialized::create()); return tmp; } }; @@ -769,26 +769,28 @@ class WeakHeapPtr : public ReadBarriered, using ReadBarriered::value; public: - WeakHeapPtr() : ReadBarriered(JS::SafelyInitialized()) {} + WeakHeapPtr() : ReadBarriered(JS::SafelyInitialized::create()) {} // It is okay to add barriers implicitly. MOZ_IMPLICIT WeakHeapPtr(const T& v) : ReadBarriered(v) { - this->post(JS::SafelyInitialized(), v); + this->post(JS::SafelyInitialized::create(), v); } // The copy constructor creates a new weak edge but the wrapped pointer does // not escape, so no read barrier is necessary. explicit WeakHeapPtr(const WeakHeapPtr& other) : ReadBarriered(other) { - this->post(JS::SafelyInitialized(), value); + this->post(JS::SafelyInitialized::create(), value); } // Move retains the lifetime status of the source edge, so does not fire // the read barrier of the defunct edge. WeakHeapPtr(WeakHeapPtr&& other) : ReadBarriered(other.release()) { - this->post(JS::SafelyInitialized(), value); + this->post(JS::SafelyInitialized::create(), value); } - ~WeakHeapPtr() { this->post(this->value, JS::SafelyInitialized()); } + ~WeakHeapPtr() { + this->post(this->value, JS::SafelyInitialized::create()); + } WeakHeapPtr& operator=(const WeakHeapPtr& v) { AssertTargetIsNotGray(v.value); @@ -832,7 +834,7 @@ class WeakHeapPtr : public ReadBarriered, T release() { T tmp = value; - set(JS::SafelyInitialized()); + set(JS::SafelyInitialized::create()); return tmp; } }; @@ -844,7 +846,7 @@ class WeakHeapPtr : public ReadBarriered, template class UnsafeBarePtr : public BarrieredBase { public: - UnsafeBarePtr() : BarrieredBase(JS::SafelyInitialized()) {} + UnsafeBarePtr() : BarrieredBase(JS::SafelyInitialized::create()) {} MOZ_IMPLICIT UnsafeBarePtr(T v) : BarrieredBase(v) {} const T& get() const { return this->value; } void set(T newValue) { this->value = newValue; } diff --git a/js/src/gc/MaybeRooted.h b/js/src/gc/MaybeRooted.h index a6518159f424..fd615a1efbee 100644 --- a/js/src/gc/MaybeRooted.h +++ b/js/src/gc/MaybeRooted.h @@ -32,7 +32,8 @@ class MOZ_RAII FakeRooted : public RootedOperations> { public: using ElementType = T; - explicit FakeRooted(JSContext* cx) : ptr(JS::SafelyInitialized()) {} + explicit FakeRooted(JSContext* cx) + : ptr(JS::SafelyInitialized::create()) {} FakeRooted(JSContext* cx, T initial) : ptr(initial) {} diff --git a/js/src/gc/NurseryAwareHashMap.h b/js/src/gc/NurseryAwareHashMap.h index a4132d81260f..79f3142484b9 100644 --- a/js/src/gc/NurseryAwareHashMap.h +++ b/js/src/gc/NurseryAwareHashMap.h @@ -23,7 +23,8 @@ namespace detail { template class UnsafeBareWeakHeapPtr : public ReadBarriered { public: - UnsafeBareWeakHeapPtr() : ReadBarriered(JS::SafelyInitialized()) {} + UnsafeBareWeakHeapPtr() + : ReadBarriered(JS::SafelyInitialized::create()) {} MOZ_IMPLICIT UnsafeBareWeakHeapPtr(const T& v) : ReadBarriered(v) {} explicit UnsafeBareWeakHeapPtr(const UnsafeBareWeakHeapPtr& v) : ReadBarriered(v) {} @@ -42,7 +43,7 @@ class UnsafeBareWeakHeapPtr : public ReadBarriered { const T get() const { if (!InternalBarrierMethods::isMarkable(this->value)) { - return JS::SafelyInitialized(); + return JS::SafelyInitialized::create(); } this->read(); return this->value; diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index c94acf967f00..ce777b3efdb6 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -1787,10 +1787,10 @@ class ReservedRooted : public RootedOperations> { } explicit ReservedRooted(Rooted* root) : savedRoot(root) { - *root = JS::SafelyInitialized(); + *root = JS::SafelyInitialized::create(); } - ~ReservedRooted() { *savedRoot = JS::SafelyInitialized(); } + ~ReservedRooted() { *savedRoot = JS::SafelyInitialized::create(); } void set(const T& p) const { *savedRoot = p; } operator Handle() { return *savedRoot; }