Bug 1619177 - Do not check for extra strict mode in ObjectOpResult. r=jorendorff

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

--HG--
extra : moz-landing-system : lando

js/public/Class.h

@@ -173,70 +173,55 @@ class ObjectOpResult {
   }
 
   /*
-   * Report an error or warning if necessary; return true to proceed and
-   * false if an error was reported. Call this when failure should cause
-   * a warning if extraWarnings are enabled.
+   * Report an error if necessary; return true to proceed and
+   * false if an error was reported.
    *
    * The precise rules are like this:
    *
    * -   If ok(), then we succeeded. Do nothing and return true.
-   * -   Otherwise, if |strict| is true, or if cx has both extraWarnings and
-   *     werrorOption enabled, throw a TypeError and return false.
-   * -   Otherwise, if cx has extraWarnings enabled, emit a warning and
-   *     return true.
+   * -   Otherwise, if |strict| is true, throw a TypeError and return false.
    * -   Otherwise, do nothing and return true.
    */
-  bool checkStrictErrorOrWarning(JSContext* cx, HandleObject obj, HandleId id,
-                                 bool strict) {
-    if (ok()) {
+  bool checkStrictModeError(JSContext* cx, HandleObject obj, HandleId id,
+                            bool strict) {
+    if (ok() || !strict) {
       return true;
     }
-    return reportStrictErrorOrWarning(cx, obj, id, strict);
+    return reportError(cx, obj, id);
   }
 
   /*
-   * The same as checkStrictErrorOrWarning(cx, id, strict), except the
+   * The same as checkStrictModeError(cx, id, strict), except the
    * operation is not associated with a particular property id. This is
    * used for [[PreventExtensions]] and [[SetPrototypeOf]]. failureCode()
    * must not be an error that has "{0}" in the error message.
    */
-  bool checkStrictErrorOrWarning(JSContext* cx, HandleObject obj, bool strict) {
-    return ok() || reportStrictErrorOrWarning(cx, obj, strict);
+  bool checkStrictModeError(JSContext* cx, HandleObject obj, bool strict) {
+    if (ok() || !strict) {
+      return true;
+    }
+    return reportError(cx, obj);
   }
 
   /* Throw a TypeError. Call this only if !ok(). */
-  bool reportError(JSContext* cx, HandleObject obj, HandleId id) {
-    return reportStrictErrorOrWarning(cx, obj, id, true);
-  }
+  bool reportError(JSContext* cx, HandleObject obj, HandleId id);
 
   /*
    * The same as reportError(cx, obj, id), except the operation is not
    * associated with a particular property id.
    */
-  bool reportError(JSContext* cx, HandleObject obj) {
-    return reportStrictErrorOrWarning(cx, obj, true);
-  }
+  bool reportError(JSContext* cx, HandleObject obj);
 
-  /* Helper function for checkStrictErrorOrWarning's slow path. */
-  JS_PUBLIC_API bool reportStrictErrorOrWarning(JSContext* cx, HandleObject obj,
-                                                HandleId id, bool strict);
-  JS_PUBLIC_API bool reportStrictErrorOrWarning(JSContext* cx, HandleObject obj,
-                                                bool strict);
-
-  /*
-   * Convenience method. Return true if ok() or if strict is false; otherwise
-   * throw a TypeError and return false.
-   */
+  // Convenience method. Return true if ok(); otherwise throw a TypeError
+  // and return false.
   bool checkStrict(JSContext* cx, HandleObject obj, HandleId id) {
-    return checkStrictErrorOrWarning(cx, obj, id, true);
+    return checkStrictModeError(cx, obj, id, true);
   }
 
-  /*
-   * Convenience method. The same as checkStrict(cx, id), except the
-   * operation is not associated with a particular property id.
-   */
+  // Convenience method. The same as checkStrict(cx, obj, id), except the
+  // operation is not associated with a particular property id.
   bool checkStrict(JSContext* cx, HandleObject obj) {
-    return checkStrictErrorOrWarning(cx, obj, true);
+    return checkStrictModeError(cx, obj, true);
   }
 };
 

js/src/jit/BaselineIC.cpp

@@ -2725,8 +2725,7 @@ bool DoSetPropFallback(JSContext* cx, BaselineFrame* frame,
 
     ObjectOpResult result;
     if (!SetProperty(cx, obj, id, rhs, lhs, result) ||
-        !result.checkStrictErrorOrWarning(cx, obj, id,
-                                          op == JSOp::StrictSetProp)) {
+        !result.checkStrictModeError(cx, obj, id, op == JSOp::StrictSetProp)) {
       return false;
     }
   }

js/src/jit/VMFunctions.cpp

@@ -561,7 +561,7 @@ bool SetArrayLength(JSContext* cx, HandleObject obj, HandleValue value,
     MOZ_ALWAYS_TRUE(result.fail(JSMSG_READ_ONLY));
   }
 
-  return result.checkStrictErrorOrWarning(cx, obj, id, strict);
+  return result.checkStrictModeError(cx, obj, id, strict);
 }
 
 bool CharCodeAt(JSContext* cx, HandleString str, int32_t index,
@@ -628,7 +628,7 @@ bool SetProperty(JSContext* cx, HandleObject obj, HandlePropertyName name,
       return false;
     }
   }
-  return result.checkStrictErrorOrWarning(cx, obj, id, strict);
+  return result.checkStrictModeError(cx, obj, id, strict);
 }
 
 bool InterruptCheck(JSContext* cx) {

js/src/jsapi.cpp

@@ -162,20 +162,18 @@ static bool ErrorTakesObjectArgument(unsigned msg) {
   return argCount == 2;
 }
 
-JS_PUBLIC_API bool JS::ObjectOpResult::reportStrictErrorOrWarning(
-    JSContext* cx, HandleObject obj, HandleId id, bool strict) {
+bool JS::ObjectOpResult::reportError(JSContext* cx, HandleObject obj,
+                                     HandleId id) {
   static_assert(unsigned(OkCode) == unsigned(JSMSG_NOT_AN_ERROR),
                 "unsigned value of OkCode must not be an error code");
   MOZ_ASSERT(code_ != Uninitialized);
   MOZ_ASSERT(!ok());
   cx->check(obj);
 
-  unsigned flags =
-      strict ? JSREPORT_ERROR : (JSREPORT_WARNING | JSREPORT_STRICT);
   if (code_ == JSMSG_OBJECT_NOT_EXTENSIBLE) {
     RootedValue val(cx, ObjectValue(*obj));
-    return ReportValueErrorFlags(cx, flags, code_, JSDVG_IGNORE_STACK, val,
-                                 nullptr, nullptr, nullptr);
+    return ReportValueErrorFlags(cx, JSREPORT_ERROR, code_, JSDVG_IGNORE_STACK,
+                                 val, nullptr, nullptr, nullptr);
   }
 
   if (ErrorTakesArguments(code_)) {
@@ -193,34 +191,32 @@ JS_PUBLIC_API bool JS::ObjectOpResult::reportStrictErrorOrWarning(
           return false;
         }
       }
-      return ReportValueErrorFlags(cx, flags, code_, JSDVG_IGNORE_STACK, val,
-                                   nullptr, propName.get(), nullptr);
+      return ReportValueErrorFlags(cx, JSREPORT_ERROR, code_,
+                                   JSDVG_IGNORE_STACK, val, nullptr,
+                                   propName.get(), nullptr);
     }
 
     if (ErrorTakesObjectArgument(code_)) {
       return JS_ReportErrorFlagsAndNumberUTF8(
-          cx, flags, GetErrorMessage, nullptr, code_, obj->getClass()->name,
-          propName.get());
+          cx, JSREPORT_ERROR, GetErrorMessage, nullptr, code_,
+          obj->getClass()->name, propName.get());
     }
 
-    return JS_ReportErrorFlagsAndNumberUTF8(cx, flags, GetErrorMessage, nullptr,
-                                            code_, propName.get());
+    return JS_ReportErrorFlagsAndNumberUTF8(cx, JSREPORT_ERROR, GetErrorMessage,
+                                            nullptr, code_, propName.get());
   }
-  return JS_ReportErrorFlagsAndNumberASCII(cx, flags, GetErrorMessage, nullptr,
-                                           code_);
+  return JS_ReportErrorFlagsAndNumberASCII(cx, JSREPORT_ERROR, GetErrorMessage,
+                                           nullptr, code_);
 }
 
-JS_PUBLIC_API bool JS::ObjectOpResult::reportStrictErrorOrWarning(
-    JSContext* cx, HandleObject obj, bool strict) {
+bool JS::ObjectOpResult::reportError(JSContext* cx, HandleObject obj) {
   MOZ_ASSERT(code_ != Uninitialized);
   MOZ_ASSERT(!ok());
   MOZ_ASSERT(!ErrorTakesArguments(code_));
   cx->check(obj);
 
-  unsigned flags =
-      strict ? JSREPORT_ERROR : (JSREPORT_WARNING | JSREPORT_STRICT);
-  return JS_ReportErrorFlagsAndNumberASCII(cx, flags, GetErrorMessage, nullptr,
-                                           code_);
+  return JS_ReportErrorFlagsAndNumberASCII(cx, JSREPORT_ERROR, GetErrorMessage,
+                                           nullptr, code_);
 }
 
 JS_PUBLIC_API bool JS::ObjectOpResult::failCantRedefineProp() {

js/src/proxy/Proxy.cpp

@@ -397,7 +397,7 @@ bool js::ProxySetProperty(JSContext* cx, HandleObject proxy, HandleId id,
   if (!Proxy::setInternal(cx, proxy, id, val, receiver, result)) {
     return false;
   }
-  return result.checkStrictErrorOrWarning(cx, proxy, id, strict);
+  return result.checkStrictModeError(cx, proxy, id, strict);
 }
 
 bool js::ProxySetPropertyByValue(JSContext* cx, HandleObject proxy,
@@ -413,7 +413,7 @@ bool js::ProxySetPropertyByValue(JSContext* cx, HandleObject proxy,
   if (!Proxy::setInternal(cx, proxy, id, val, receiver, result)) {
     return false;
   }
-  return result.checkStrictErrorOrWarning(cx, proxy, id, strict);
+  return result.checkStrictModeError(cx, proxy, id, strict);
 }
 
 bool Proxy::getOwnEnumerablePropertyKeys(JSContext* cx, HandleObject proxy,

js/src/tests/non262/extensions/8.12.5-01.js

@@ -57,33 +57,6 @@ try
   {
     errors.push(e);
   }
-
-  options("strict");
-  options("werror");
-  try
-  {
-    test1();
-    errors.push("strict+werror didn't make test1 fail");
-  }
-  catch (e)
-  {
-    if (!(e instanceof TypeError))
-      errors.push("test1 with strict+werror failed without a TypeError: " + e);
-  }
-
-  try
-  {
-    test2();
-    errors.push("strict+werror didn't make test2 fail");
-  }
-  catch (e)
-  {
-    if (!(e instanceof TypeError))
-      errors.push("test2 with strict+werror failed without a TypeError: " + e);
-  }
-
-  options("strict");
-  options("werror");
 }
 catch (e)
 {

js/src/vm/Interpreter-inl.h

@@ -308,7 +308,7 @@ inline bool SetNameOperation(JSContext* cx, JSScript* script, jsbytecode* pc,
   } else {
     ok = SetProperty(cx, env, id, val, receiver, result);
   }
-  return ok && result.checkStrictErrorOrWarning(cx, env, id, strict);
+  return ok && result.checkStrictModeError(cx, env, id, strict);
 }
 
 inline void InitGlobalLexicalOperation(JSContext* cx,

js/src/vm/Interpreter.cpp

@@ -270,8 +270,7 @@ static bool SetPropertyOperation(JSContext* cx, JSOp op, HandleValue lval,
 
   ObjectOpResult result;
   return SetProperty(cx, obj, id, rval, lval, result) &&
-         result.checkStrictErrorOrWarning(cx, obj, id,
-                                          op == JSOp::StrictSetProp);
+         result.checkStrictModeError(cx, obj, id, op == JSOp::StrictSetProp);
 }
 
 JSFunction* js::MakeDefaultConstructor(JSContext* cx, HandleScript script,
@@ -1593,7 +1592,7 @@ static MOZ_ALWAYS_INLINE bool SetObjectElementOperation(
 
   ObjectOpResult result;
   return SetProperty(cx, obj, id, value, receiver, result) &&
-         result.checkStrictErrorOrWarning(cx, obj, id, strict);
+         result.checkStrictModeError(cx, obj, id, strict);
 }
 
 /*
@@ -5347,5 +5346,5 @@ bool js::SetPropertySuper(JSContext* cx, HandleObject obj, HandleValue receiver,
     return false;
   }
 
-  return result.checkStrictErrorOrWarning(cx, obj, id, strict);
+  return result.checkStrictModeError(cx, obj, id, strict);
 }

js/src/vm/NativeObject.cpp

@@ -2119,7 +2119,7 @@ bool js::AddOrUpdateSparseElementHelper(JSContext* cx, HandleArrayObject obj,
   RootedValue receiver(cx, ObjectValue(*obj));
   JS::ObjectOpResult result;
   return SetProperty(cx, obj, id, v, receiver, result) &&
-         result.checkStrictErrorOrWarning(cx, obj, id, strict);
+         result.checkStrictModeError(cx, obj, id, strict);
 }
 
 /*** [[HasProperty]] ********************************************************/

js/src/vm/ObjectOperations-inl.h

@@ -331,7 +331,7 @@ inline bool PutProperty(JSContext* cx, JS::Handle<JSObject*> obj,
   JS::Rooted<JS::Value> receiver(cx, JS::ObjectValue(*obj));
   JS::ObjectOpResult result;
   return SetProperty(cx, obj, id, v, receiver, result) &&
-         result.checkStrictErrorOrWarning(cx, obj, id, strict);
+         result.checkStrictModeError(cx, obj, id, strict);
 }
 
 /*