Bug 1471931 - Part 5: Use MakeUnique in more places and replace manual js_delete with UniquePtr. r=sfink
authorAndré Bargull <andre.bargull@gmail.com>
Fri, 29 Jun 2018 02:33:51 -0700
changeset 424446 266767b68e8c4552a2f7a602ca103883d45e91de
parent 424445 4e320fc1cbb8c73e868b4bbbe6dee924eed384bc
child 424447 5a684dbbde3de9881bc9af78cbe93770b94dc28b
push id104819
push userdluca@mozilla.com
push dateFri, 29 Jun 2018 20:35:29 +0000
treeherdermozilla-inbound@266767b68e8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfink
bugs1471931
milestone63.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 1471931 - Part 5: Use MakeUnique in more places and replace manual js_delete with UniquePtr. r=sfink
js/public/UbiNodeShortestPaths.h
js/src/jit/CodeGenerator.cpp
js/src/jit/Ion.cpp
js/src/jit/arm/Simulator-arm.cpp
js/src/jit/arm64/vixl/MozSimulator-vixl.cpp
js/src/jit/mips32/Simulator-mips32.cpp
js/src/jit/mips64/Simulator-mips64.cpp
js/src/vm/Caches.cpp
js/src/vm/Shape.cpp
js/src/vm/SharedImmutableStringsCache.h
js/src/vm/UbiNode.cpp
js/src/vm/UbiNodeCensus.cpp
js/src/vm/UbiNodeShortestPaths.cpp
js/src/wasm/WasmModule.cpp
--- a/js/public/UbiNodeShortestPaths.h
+++ b/js/public/UbiNodeShortestPaths.h
@@ -8,32 +8,33 @@
 #define js_UbiNodeShortestPaths_h
 
 #include "mozilla/Attributes.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/Move.h"
 
 #include "js/AllocPolicy.h"
 #include "js/UbiNodeBreadthFirst.h"
+#include "js/UniquePtr.h"
 #include "js/Vector.h"
 
 namespace JS {
 namespace ubi {
 
 /**
  * A back edge along a path in the heap graph.
  */
 struct JS_PUBLIC_API(BackEdge)
 {
   private:
     Node predecessor_;
     EdgeName name_;
 
   public:
-    using Ptr = mozilla::UniquePtr<BackEdge, JS::DeletePolicy<BackEdge>>;
+    using Ptr = js::UniquePtr<BackEdge>;
 
     BackEdge() : predecessor_(), name_(nullptr) { }
 
     MOZ_MUST_USE bool init(const Node& predecessor, Edge& edge) {
         MOZ_ASSERT(!predecessor_);
         MOZ_ASSERT(!name_);
 
         predecessor_ = predecessor;
@@ -142,17 +143,17 @@ struct JS_PUBLIC_API(ShortestPaths)
             } else {
                 auto ptr = shortestPaths.paths_.lookup(edge.referent);
                 MOZ_ASSERT(ptr,
                            "This isn't the first time we have seen the target node `edge.referent`. "
                            "We should have inserted it into shortestPaths.paths_ the first time we "
                            "saw it.");
 
                 if (ptr->value().length() < shortestPaths.maxNumPaths_) {
-                    BackEdge::Ptr thisBackEdge(js_new<BackEdge>());
+                    auto thisBackEdge = js::MakeUnique<BackEdge>();
                     if (!thisBackEdge || !thisBackEdge->init(origin, edge))
                         return false;
                     ptr->value().infallibleAppend(std::move(thisBackEdge));
                     totalPathsRecorded++;
                 }
             }
 
             MOZ_ASSERT(totalPathsRecorded <= totalMaxPathsToRecord);
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -5379,17 +5379,17 @@ CodeGenerator::maybeCreateScriptCounts()
     // currently incompatible with wasm codegen for two reasons: (1) wasm code
     // must be serializable and script count codegen bakes in absolute
     // addresses, (2) wasm code does not have a JSScript with which to associate
     // code coverage data.
     JSScript* script = gen->info().script();
     if (!script)
         return nullptr;
 
-    UniquePtr<IonScriptCounts> counts(js_new<IonScriptCounts>());
+    auto counts = MakeUnique<IonScriptCounts>();
     if (!counts || !counts->init(graph.numBlocks()))
         return nullptr;
 
     for (size_t i = 0; i < graph.numBlocks(); i++) {
         MBasicBlock* block = graph.getBlock(i)->mir();
 
         uint32_t offset = 0;
         char* description = nullptr;
--- a/js/src/jit/Ion.cpp
+++ b/js/src/jit/Ion.cpp
@@ -1867,26 +1867,24 @@ GenerateLIR(MIRGenerator* mir)
 }
 
 CodeGenerator*
 GenerateCode(MIRGenerator* mir, LIRGraph* lir)
 {
     TraceLoggerThread* logger = TraceLoggerForCurrentThread();
     AutoTraceLog log(logger, TraceLogger_GenerateCode);
 
-    CodeGenerator* codegen = js_new<CodeGenerator>(mir, lir);
+    auto codegen = MakeUnique<CodeGenerator>(mir, lir);
     if (!codegen)
         return nullptr;
 
-    if (!codegen->generate()) {
-        js_delete(codegen);
+    if (!codegen->generate())
         return nullptr;
-    }
-
-    return codegen;
+
+    return codegen.release();
 }
 
 CodeGenerator*
 CompileBackEnd(MIRGenerator* mir)
 {
     // Everything in CompileBackEnd can potentially run on a helper thread.
     AutoEnterIonCompilation enter(mir->safeForMinorGC());
     AutoSpewEndFunction spewEndFunction(mir);
--- a/js/src/jit/arm/Simulator-arm.cpp
+++ b/js/src/jit/arm/Simulator-arm.cpp
@@ -33,16 +33,17 @@
 #include "mozilla/FloatingPoint.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MathAlgorithms.h"
 #include "mozilla/Unused.h"
 
 #include "jit/arm/Assembler-arm.h"
 #include "jit/arm/disasm/Constants-arm.h"
 #include "jit/AtomicOperations.h"
+#include "js/UniquePtr.h"
 #include "js/Utility.h"
 #include "threading/LockGuard.h"
 #include "vm/Runtime.h"
 #include "vm/SharedMem.h"
 #include "wasm/WasmInstance.h"
 #include "wasm/WasmSignalHandlers.h"
 
 extern "C" {
@@ -410,33 +411,31 @@ mozilla::Atomic<size_t, mozilla::Release
     SimulatorProcess::ICacheCheckingDisableCount(1); // Checking is disabled by default.
 SimulatorProcess* SimulatorProcess::singleton_ = nullptr;
 
 int64_t Simulator::StopSimAt = -1L;
 
 Simulator*
 Simulator::Create(JSContext* cx)
 {
-    Simulator* sim = js_new<Simulator>(cx);
+    auto sim = MakeUnique<Simulator>(cx);
     if (!sim)
         return nullptr;
 
-    if (!sim->init()) {
-        js_delete(sim);
+    if (!sim->init())
         return nullptr;
-    }
 
     char* stopAtStr = getenv("ARM_SIM_STOP_AT");
     int64_t stopAt;
     if (stopAtStr && sscanf(stopAtStr, "%lld", &stopAt) == 1) {
         fprintf(stderr, "\nStopping simulation at icount %lld\n", stopAt);
         Simulator::StopSimAt = stopAt;
     }
 
-    return sim;
+    return sim.release();
 }
 
 void
 Simulator::Destroy(Simulator* sim)
 {
     js_delete(sim);
 }
 
--- a/js/src/jit/arm64/vixl/MozSimulator-vixl.cpp
+++ b/js/src/jit/arm64/vixl/MozSimulator-vixl.cpp
@@ -24,16 +24,17 @@
 // OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include "mozilla/DebugOnly.h"
 
 #include "jit/arm64/vixl/Debugger-vixl.h"
 #include "jit/arm64/vixl/Simulator-vixl.h"
 #include "jit/IonTypes.h"
+#include "js/UniquePtr.h"
 #include "js/Utility.h"
 #include "threading/LockGuard.h"
 #include "vm/Runtime.h"
 #include "wasm/WasmInstance.h"
 #include "wasm/WasmProcess.h"
 #include "wasm/WasmSignalHandlers.h"
 
 js::jit::SimulatorProcess* js::jit::SimulatorProcess::singleton_ = nullptr;
@@ -163,29 +164,27 @@ Simulator* Simulator::Current() {
 Simulator* Simulator::Create(JSContext* cx) {
   Decoder *decoder = js_new<vixl::Decoder>();
   if (!decoder)
     return nullptr;
 
   // FIXME: This just leaks the Decoder object for now, which is probably OK.
   // FIXME: We should free it at some point.
   // FIXME: Note that it can't be stored in the SimulatorRuntime due to lifetime conflicts.
-  Simulator *sim;
+  js::UniquePtr<Simulator> sim;
   if (getenv("USE_DEBUGGER") != nullptr)
-    sim = js_new<Debugger>(cx, decoder, stdout);
+    sim.reset(js_new<Debugger>(cx, decoder, stdout));
   else
-    sim = js_new<Simulator>(cx, decoder, stdout);
+    sim.reset(js_new<Simulator>(cx, decoder, stdout));
 
   // Check if Simulator:init ran out of memory.
-  if (sim && sim->oom()) {
-    js_delete(sim);
+  if (sim && sim->oom())
     return nullptr;
-  }
 
-  return sim;
+  return sim.release();
 }
 
 
 void Simulator::Destroy(Simulator* sim) {
   js_delete(sim);
 }
 
 
--- a/js/src/jit/mips32/Simulator-mips32.cpp
+++ b/js/src/jit/mips32/Simulator-mips32.cpp
@@ -32,16 +32,17 @@
 #include "mozilla/FloatingPoint.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MathAlgorithms.h"
 
 #include <float.h>
 
 #include "jit/AtomicOperations.h"
 #include "jit/mips32/Assembler-mips32.h"
+#include "js/UniquePtr.h"
 #include "js/Utility.h"
 #include "vm/Runtime.h"
 #include "wasm/WasmInstance.h"
 #include "wasm/WasmSignalHandlers.h"
 
 #define I8(v)   static_cast<int8_t>(v)
 #define I16(v)  static_cast<int16_t>(v)
 #define U16(v)  static_cast<uint16_t>(v)
@@ -519,33 +520,31 @@ mozilla::Atomic<size_t, mozilla::Release
     SimulatorProcess::ICacheCheckingDisableCount(1); // Checking is disabled by default.
 SimulatorProcess* SimulatorProcess::singleton_ = nullptr;
 
 int Simulator::StopSimAt = -1;
 
 Simulator*
 Simulator::Create(JSContext* cx)
 {
-    Simulator* sim = js_new<Simulator>();
+    auto sim = MakeUnique<Simulator>();
     if (!sim)
         return nullptr;
 
-    if (!sim->init()) {
-        js_delete(sim);
+    if (!sim->init())
         return nullptr;
-    }
 
     char* stopAtStr = getenv("MIPS_SIM_STOP_AT");
     int64_t stopAt;
     if (stopAtStr && sscanf(stopAtStr, "%lld", &stopAt) == 1) {
         fprintf(stderr, "\nStopping simulation at icount %lld\n", stopAt);
         Simulator::StopSimAt = stopAt;
     }
 
-    return sim;
+    return sim.release();
 }
 
 void
 Simulator::Destroy(Simulator* sim)
 {
     js_delete(sim);
 }
 
--- a/js/src/jit/mips64/Simulator-mips64.cpp
+++ b/js/src/jit/mips64/Simulator-mips64.cpp
@@ -35,16 +35,17 @@
 #include "mozilla/Likely.h"
 #include "mozilla/MathAlgorithms.h"
 
 #include <float.h>
 #include <limits>
 
 #include "jit/AtomicOperations.h"
 #include "jit/mips64/Assembler-mips64.h"
+#include "js/UniquePtr.h"
 #include "js/Utility.h"
 #include "threading/LockGuard.h"
 #include "vm/Runtime.h"
 #include "wasm/WasmInstance.h"
 #include "wasm/WasmSignalHandlers.h"
 
 #define I8(v)   static_cast<int8_t>(v)
 #define I16(v)  static_cast<int16_t>(v)
@@ -554,33 +555,31 @@ mozilla::Atomic<size_t, mozilla::Release
     SimulatorProcess::ICacheCheckingDisableCount(1);  // Checking is disabled by default.
 SimulatorProcess* SimulatorProcess::singleton_ = nullptr;
 
 int64_t Simulator::StopSimAt = -1;
 
 Simulator *
 Simulator::Create(JSContext* cx)
 {
-    Simulator* sim = js_new<Simulator>();
+    auto sim = MakeUnique<Simulator>();
     if (!sim)
         return nullptr;
 
-    if (!sim->init()) {
-        js_delete(sim);
+    if (!sim->init())
         return nullptr;
-    }
 
     int64_t stopAt;
     char* stopAtStr = getenv("MIPS_SIM_STOP_AT");
     if (stopAtStr && sscanf(stopAtStr, "%" PRIi64, &stopAt) == 1) {
         fprintf(stderr, "\nStopping simulation at icount %" PRIi64 "\n", stopAt);
         Simulator::StopSimAt = stopAt;
     }
 
-    return sim;
+    return sim.release();
 }
 
 void
 Simulator::Destroy(Simulator* sim)
 {
     js_delete(sim);
 }
 
--- a/js/src/vm/Caches.cpp
+++ b/js/src/vm/Caches.cpp
@@ -12,17 +12,17 @@ using namespace js;
 
 using mozilla::PodZero;
 
 MathCache*
 RuntimeCaches::createMathCache(JSContext* cx)
 {
     MOZ_ASSERT(!mathCache_);
 
-    UniquePtr<MathCache> newMathCache(js_new<MathCache>());
+    auto newMathCache = MakeUnique<MathCache>();
     if (!newMathCache) {
         ReportOutOfMemory(cx);
         return nullptr;
     }
 
     mathCache_ = std::move(newMathCache);
     return mathCache_.get();
 }
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -11,16 +11,17 @@
 #include "mozilla/MathAlgorithms.h"
 #include "mozilla/PodOperations.h"
 
 #include "gc/FreeOp.h"
 #include "gc/HashUtil.h"
 #include "gc/Policy.h"
 #include "gc/PublicIterators.h"
 #include "js/HashTable.h"
+#include "js/UniquePtr.h"
 #include "util/Text.h"
 #include "vm/JSAtom.h"
 #include "vm/JSContext.h"
 #include "vm/JSObject.h"
 
 #include "vm/Caches-inl.h"
 #include "vm/JSContext-inl.h"
 #include "vm/JSObject-inl.h"
@@ -1623,25 +1624,23 @@ MOZ_ALWAYS_INLINE bool
 ShapeHasher::match(const Key k, const Lookup& l)
 {
     return k->matches(l);
 }
 
 static KidsHash*
 HashChildren(Shape* kid1, Shape* kid2)
 {
-    KidsHash* hash = js_new<KidsHash>();
-    if (!hash || !hash->init(2)) {
-        js_delete(hash);
+    auto hash = MakeUnique<KidsHash>();
+    if (!hash || !hash->init(2))
         return nullptr;
-    }
 
     hash->putNewInfallible(StackShape(kid1), kid1);
     hash->putNewInfallible(StackShape(kid2), kid2);
-    return hash;
+    return hash.release();
 }
 
 bool
 PropertyTree::insertChild(JSContext* cx, Shape* parent, Shape* child)
 {
     MOZ_ASSERT(!parent->inDictionary());
     MOZ_ASSERT(!child->parent);
     MOZ_ASSERT(!child->inDictionary());
--- a/js/src/vm/SharedImmutableStringsCache.h
+++ b/js/src/vm/SharedImmutableStringsCache.h
@@ -11,16 +11,17 @@
 #include "mozilla/UniquePtr.h"
 
 #include <cstring>
 #include <new> // for placement new
 
 #include "builtin/String.h"
 
 #include "js/HashTable.h"
+#include "js/UniquePtr.h"
 #include "js/Utility.h"
 
 #include "threading/ExclusiveData.h"
 
 #include "vm/MutexIDs.h"
 
 namespace js {
 
@@ -234,28 +235,28 @@ class SharedImmutableStringsCache
         friend class SharedImmutableString;
 
         OwnedChars chars_;
         size_t length_;
 
       public:
         mutable size_t refcount;
 
-        using Ptr = mozilla::UniquePtr<StringBox, JS::DeletePolicy<StringBox>>;
+        using Ptr = js::UniquePtr<StringBox>;
 
         StringBox(OwnedChars&& chars, size_t length)
           : chars_(std::move(chars))
           , length_(length)
           , refcount(0)
         {
             MOZ_ASSERT(chars_);
         }
 
         static Ptr Create(OwnedChars&& chars, size_t length) {
-            return Ptr(js_new<StringBox>(std::move(chars), length));
+            return js::MakeUnique<StringBox>(std::move(chars), length);
         }
 
         StringBox(const StringBox&) = delete;
         StringBox& operator=(const StringBox&) = delete;
 
         ~StringBox() {
             MOZ_RELEASE_ASSERT(refcount == 0,
                                "There are `SharedImmutable[TwoByte]String`s outliving their "
--- a/js/src/vm/UbiNode.cpp
+++ b/js/src/vm/UbiNode.cpp
@@ -328,24 +328,24 @@ template JS::Zone* TracerConcrete<JS::Sy
 #ifdef ENABLE_BIGINT
 template JS::Zone* TracerConcrete<BigInt>::zone() const;
 #endif
 template JS::Zone* TracerConcrete<JSString>::zone() const;
 
 template<typename Referent>
 UniquePtr<EdgeRange>
 TracerConcrete<Referent>::edges(JSContext* cx, bool wantNames) const {
-    UniquePtr<SimpleEdgeRange, JS::DeletePolicy<SimpleEdgeRange>> range(js_new<SimpleEdgeRange>());
+    auto range = js::MakeUnique<SimpleEdgeRange>();
     if (!range)
         return nullptr;
 
     if (!range->init(cx->runtime(), ptr, JS::MapTypeToTraceKind<Referent>::kind, wantNames))
         return nullptr;
 
-    return UniquePtr<EdgeRange>(range.release());
+    return range;
 }
 
 template UniquePtr<EdgeRange> TracerConcrete<JSScript>::edges(JSContext* cx, bool wantNames) const;
 template UniquePtr<EdgeRange> TracerConcrete<js::LazyScript>::edges(JSContext* cx, bool wantNames) const;
 template UniquePtr<EdgeRange> TracerConcrete<js::Shape>::edges(JSContext* cx, bool wantNames) const;
 template UniquePtr<EdgeRange> TracerConcrete<js::BaseShape>::edges(JSContext* cx, bool wantNames) const;
 template UniquePtr<EdgeRange> TracerConcrete<js::ObjectGroup>::edges(JSContext* cx, bool wantNames) const;
 template UniquePtr<EdgeRange> TracerConcrete<js::RegExpShared>::edges(JSContext* cx, bool wantNames) const;
@@ -551,13 +551,13 @@ RootList::addRoot(Node node, const char1
     return edges.append(Edge(name.release(), node));
 }
 
 const char16_t Concrete<RootList>::concreteTypeName[] = u"JS::ubi::RootList";
 
 UniquePtr<EdgeRange>
 Concrete<RootList>::edges(JSContext* cx, bool wantNames) const {
     MOZ_ASSERT_IF(wantNames, get().wantNames);
-    return UniquePtr<EdgeRange>(js_new<PreComputedEdgeRange>(get().edges));
+    return js::MakeUnique<PreComputedEdgeRange>(get().edges);
 }
 
 } // namespace ubi
 } // namespace JS
--- a/js/src/vm/UbiNodeCensus.cpp
+++ b/js/src/vm/UbiNodeCensus.cpp
@@ -433,17 +433,17 @@ class ByObjectClass : public CountType {
 
 CountBasePtr
 ByObjectClass::makeCount()
 {
     CountBasePtr otherCount(otherType->makeCount());
     if (!otherCount)
         return nullptr;
 
-    UniquePtr<Count> count(js_new<Count>(*this, otherCount));
+    auto count = js::MakeUnique<Count>(*this, otherCount);
     if (!count || !count->init())
         return nullptr;
 
     return CountBasePtr(count.release());
 }
 
 void
 ByObjectClass::traceCount(CountBase& countBase, JSTracer* trc)
@@ -527,17 +527,17 @@ class ByUbinodeType : public CountType {
     void traceCount(CountBase& countBase, JSTracer* trc) override;
     bool count(CountBase& countBase, mozilla::MallocSizeOf mallocSizeOf, const Node& node) override;
     bool report(JSContext* cx, CountBase& countBase, MutableHandleValue report) override;
 };
 
 CountBasePtr
 ByUbinodeType::makeCount()
 {
-    UniquePtr<Count> count(js_new<Count>(*this));
+    auto count = js::MakeUnique<Count>(*this);
     if (!count || !count->init())
         return nullptr;
 
     return CountBasePtr(count.release());
 }
 
 void
 ByUbinodeType::traceCount(CountBase& countBase, JSTracer* trc)
@@ -674,17 +674,17 @@ class ByAllocationStack : public CountTy
 
 CountBasePtr
 ByAllocationStack::makeCount()
 {
     CountBasePtr noStackCount(noStackType->makeCount());
     if (!noStackCount)
         return nullptr;
 
-    UniquePtr<Count> count(js_new<Count>(*this, noStackCount));
+    auto count = js::MakeUnique<Count>(*this, noStackCount);
     if (!count || !count->init())
         return nullptr;
     return CountBasePtr(count.release());
 }
 
 void
 ByAllocationStack::traceCount(CountBase& countBase, JSTracer* trc)
 {
@@ -850,17 +850,17 @@ ByFilename::makeCount()
     CountBasePtr thenCount(thenType->makeCount());
     if (!thenCount)
         return nullptr;
 
     CountBasePtr noFilenameCount(noFilenameType->makeCount());
     if (!noFilenameCount)
         return nullptr;
 
-    UniquePtr<Count> count(js_new<Count>(*this, std::move(thenCount), std::move(noFilenameCount)));
+    auto count = js::MakeUnique<Count>(*this, std::move(thenCount), std::move(noFilenameCount));
     if (!count || !count->init())
         return nullptr;
 
     return CountBasePtr(count.release());
 }
 
 void
 ByFilename::traceCount(CountBase& countBase, JSTracer* trc)
--- a/js/src/vm/UbiNodeShortestPaths.cpp
+++ b/js/src/vm/UbiNodeShortestPaths.cpp
@@ -13,17 +13,17 @@
 #include "util/Text.h"
 
 namespace JS {
 namespace ubi {
 
 JS_PUBLIC_API(BackEdge::Ptr)
 BackEdge::clone() const
 {
-    BackEdge::Ptr clone(js_new<BackEdge>());
+    auto clone = js::MakeUnique<BackEdge>();
     if (!clone)
         return nullptr;
 
     clone->predecessor_ = predecessor();
     if (name()) {
         clone->name_ = js::DuplicateString(name().get());
         if (!clone->name_)
             return nullptr;
--- a/js/src/wasm/WasmModule.cpp
+++ b/js/src/wasm/WasmModule.cpp
@@ -206,17 +206,17 @@ Module::startTier2(const CompileArgs& ar
     MOZ_ASSERT(!tiering_.lock()->active);
 
     // If a Module initiates tier-2 compilation, we must ensure that eventually
     // notifyCompilationListeners() is called. Since we must ensure
     // Tier2GeneratorTaskImpl objects are destroyed *anyway*, we use
     // ~Tier2GeneratorTaskImpl() to call notifyCompilationListeners() if it
     // hasn't been already.
 
-    UniqueTier2GeneratorTask task(js_new<Tier2GeneratorTaskImpl>(*this, args));
+    auto task = MakeUnique<Tier2GeneratorTaskImpl>(*this, args);
     if (!task)
         return;
 
     tiering_.lock()->active = true;
 
     StartOffThreadWasmTier2Generator(std::move(task));
 }