From f4d8508eef0fd5dac9c2adb3c303ffb3a6a4ea83 Mon Sep 17 00:00:00 2001
From: Jon Coppeard <jcoppeard@mozilla.com>
Date: Fri, 26 May 2023 09:28:20 +0000
Subject: [PATCH] Bug 1834537 - Part 6: Simplify GCRuntime::checkAllocatorState
 a little r=sfink

This makes allowGC checks constexpr to make it clearer that these parts are
conditionally compiled.

gcIfNeededAtAllocation always returns true so we can remove the return value.

Differential Revision: https://phabricator.services.mozilla.com/D178813
---
 js/src/gc/Allocator.cpp | 30 ++++++++++++------------------
 js/src/gc/GCRuntime.h   |  2 +-
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/js/src/gc/Allocator.cpp b/js/src/gc/Allocator.cpp
index 6756b1083f1f..d82fc5e33b06 100644
--- a/js/src/gc/Allocator.cpp
+++ b/js/src/gc/Allocator.cpp
@@ -205,13 +205,6 @@ void GCRuntime::attemptLastDitchGC(JSContext* cx) {
 
 template <AllowGC allowGC>
 bool GCRuntime::checkAllocatorState(JSContext* cx, AllocKind kind) {
-  if (allowGC) {
-    if (!gcIfNeededAtAllocation(cx)) {
-      return false;
-    }
-  }
-
-#if defined(JS_GC_ZEAL) || defined(DEBUG)
   MOZ_ASSERT_IF(cx->zone()->isAtomsZone(),
                 kind == AllocKind::ATOM || kind == AllocKind::FAT_INLINE_ATOM ||
                     kind == AllocKind::SYMBOL || kind == AllocKind::JITCODE ||
@@ -219,18 +212,21 @@ bool GCRuntime::checkAllocatorState(JSContext* cx, AllocKind kind) {
   MOZ_ASSERT_IF(!cx->zone()->isAtomsZone(),
                 kind != AllocKind::ATOM && kind != AllocKind::FAT_INLINE_ATOM);
   MOZ_ASSERT(!JS::RuntimeHeapIsBusy());
-#endif
 
-  // Crash if we perform a GC action when it is not safe.
-  if (allowGC && !cx->suppressGC) {
-    cx->verifyIsSafeToGC();
+  if constexpr (allowGC) {
+    // Crash if we could perform a GC action when it is not safe.
+    if (!cx->suppressGC) {
+      cx->verifyIsSafeToGC();
+    }
+
+    gcIfNeededAtAllocation(cx);
   }
 
-  // For testing out of memory conditions
+  // For testing out of memory conditions.
   if (js::oom::ShouldFailWithOOM()) {
-    // If we are doing a fallible allocation, percolate up the OOM
-    // instead of reporting it.
-    if (allowGC) {
+    // If we are doing a fallible allocation, percolate up the OOM instead of
+    // reporting it.
+    if constexpr (allowGC) {
       ReportOutOfMemory(cx);
     }
     return false;
@@ -239,7 +235,7 @@ bool GCRuntime::checkAllocatorState(JSContext* cx, AllocKind kind) {
   return true;
 }
 
-inline bool GCRuntime::gcIfNeededAtAllocation(JSContext* cx) {
+inline void GCRuntime::gcIfNeededAtAllocation(JSContext* cx) {
 #ifdef JS_GC_ZEAL
   if (needZealousGC()) {
     runDebugGC();
@@ -251,8 +247,6 @@ inline bool GCRuntime::gcIfNeededAtAllocation(JSContext* cx) {
   if (cx->hasAnyPendingInterrupt()) {
     gcIfRequested();
   }
-
-  return true;
 }
 
 #ifdef DEBUG
diff --git a/js/src/gc/GCRuntime.h b/js/src/gc/GCRuntime.h
index 99e531bb8426..53361140209a 100644
--- a/js/src/gc/GCRuntime.h
+++ b/js/src/gc/GCRuntime.h
@@ -668,7 +668,7 @@ class GCRuntime {
                        const AutoLockGC& lock);
 
   // Allocator internals
-  [[nodiscard]] bool gcIfNeededAtAllocation(JSContext* cx);
+  void gcIfNeededAtAllocation(JSContext* cx);
   static void* refillFreeList(JSContext* cx, AllocKind thingKind);
   void attemptLastDitchGC(JSContext* cx);
 #ifdef DEBUG
-- 
2.11.4.GIT