Revert "[runtime] Use Isolate::ThrowAt with MessageLocation"

This reverts commit eb6b4ce1.

Reason for revert: Might need rebaseline:
https://ci.chromium.org/p/v8/builders/ci/V8%20Blink%20Linux/7519

Original change's description:
> [runtime] Use Isolate::ThrowAt with MessageLocation
>
> Fix various missing source positions when reporting parse and compile
> errors. Namely this fixes missing source positions when having invalid
> module imports.
>
> - Use Isolate::ThrowAt with valid MessageLocation objects
> - Change public Isolate::Throw to no longer accept MessageLocation to
>   avoid misues
> - Introduce private Isolate::ThrowInternal that accepts MessageLocation
>
> Bug: v8:6513
> Change-Id: I3ee633c9fff8c9d361bddb37f56e28a50c280ec1
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2467839
> Commit-Queue: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Marja Hölttä <marja@chromium.org>
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#70623}

TBR=marja@chromium.org,cbruni@chromium.org,ishell@chromium.org

Change-Id: Ifa16ef8b6e5e411712fbad2e2a58fd700da12a69
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6513
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2485498Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70631}

src/execution/execution.cc

@@ -194,10 +194,10 @@ MaybeHandle<Context> NewScriptContext(Isolate* isolate,
           // If envRec.HasLexicalDeclaration(name) is true, throw a SyntaxError
           // exception.
           MessageLocation location(script, 0, 1);
-          return isolate->ThrowAt<Context>(
-              isolate->factory()->NewSyntaxError(
-                  MessageTemplate::kVarRedeclaration, name),
-              &location);
+          isolate->ThrowAt(isolate->factory()->NewSyntaxError(
+                               MessageTemplate::kVarRedeclaration, name),
+                           &location);
+          return MaybeHandle<Context>();
         }
       }
     }
@@ -216,10 +216,10 @@ MaybeHandle<Context> NewScriptContext(Isolate* isolate,
         // ES#sec-globaldeclarationinstantiation 5.d:
         // If hasRestrictedGlobal is true, throw a SyntaxError exception.
         MessageLocation location(script, 0, 1);
-        return isolate->ThrowAt<Context>(
-            isolate->factory()->NewSyntaxError(
-                MessageTemplate::kVarRedeclaration, name),
-            &location);
+        isolate->ThrowAt(isolate->factory()->NewSyntaxError(
+                             MessageTemplate::kVarRedeclaration, name),
+                         &location);
+        return MaybeHandle<Context>();
       }
 
       JSGlobalObject::InvalidatePropertyCell(global_object, name);

src/execution/isolate.cc

@@ -1412,7 +1412,7 @@ Object Isolate::StackOverflow() {
       ErrorUtils::Construct(this, fun, fun, msg, SKIP_NONE, no_caller,
                             ErrorUtils::StackTraceCollection::kSimple));
 
-  Throw(*exception);
+  Throw(*exception, nullptr);
 
 #ifdef VERIFY_HEAP
   if (FLAG_verify_heap && FLAG_stress_compaction) {
@@ -1424,7 +1424,7 @@ Object Isolate::StackOverflow() {
   return ReadOnlyRoots(heap()).exception();
 }
 
-Object Isolate::ThrowAt(Handle<JSObject> exception, MessageLocation* location) {
+void Isolate::ThrowAt(Handle<JSObject> exception, MessageLocation* location) {
   Handle<Name> key_start_pos = factory()->error_start_pos_symbol();
   Object::SetProperty(this, exception, key_start_pos,
                       handle(Smi::FromInt(location->start_pos()), this),
@@ -1445,11 +1445,11 @@ Object Isolate::ThrowAt(Handle<JSObject> exception, MessageLocation* location) {
                       Just(ShouldThrow::kThrowOnError))
       .Check();
 
-  return ThrowInternal(*exception, location);
+  Throw(*exception, location);
 }
 
 Object Isolate::TerminateExecution() {
-  return Throw(ReadOnlyRoots(this).termination_exception());
+  return Throw(ReadOnlyRoots(this).termination_exception(), nullptr);
 }
 
 void Isolate::CancelTerminateExecution() {
@@ -1579,7 +1579,7 @@ Handle<JSMessageObject> Isolate::CreateMessageOrAbort(
   return message_obj;
 }
 
-Object Isolate::ThrowInternal(Object raw_exception, MessageLocation* location) {
+Object Isolate::Throw(Object raw_exception, MessageLocation* location) {
   DCHECK(!has_pending_exception());
 
   HandleScope scope(this);

src/execution/isolate.h

@@ -818,22 +818,17 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   // Exception throwing support. The caller should use the result
   // of Throw() as its return value.
-  Object Throw(Object exception) { return ThrowInternal(exception, nullptr); }
-  Object ThrowAt(Handle<JSObject> exception, MessageLocation* location);
+  Object Throw(Object exception, MessageLocation* location = nullptr);
   Object ThrowIllegalOperation();
 
   template <typename T>
-  V8_WARN_UNUSED_RESULT MaybeHandle<T> Throw(Handle<Object> exception) {
-    Throw(*exception);
+  V8_WARN_UNUSED_RESULT MaybeHandle<T> Throw(
+      Handle<Object> exception, MessageLocation* location = nullptr) {
+    Throw(*exception, location);
     return MaybeHandle<T>();
   }
 
-  template <typename T>
-  V8_WARN_UNUSED_RESULT MaybeHandle<T> ThrowAt(Handle<JSObject> exception,
-                                               MessageLocation* location) {
-    ThrowAt(exception, location);
-    return MaybeHandle<T>();
-  }
+  void ThrowAt(Handle<JSObject> exception, MessageLocation* location);
 
   void FatalProcessOutOfHeapMemory(const char* location) {
     heap()->FatalProcessOutOfMemory(location);
@@ -1720,9 +1715,6 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
 
   void AddCrashKeysForIsolateAndHeapPointers();
 
-  // Returns the Exception sentinel.
-  Object ThrowInternal(Object exception, MessageLocation* location);
-
   // This class contains a collection of data accessible from both C++ runtime
   // and compiled code (including assembly stubs, builtins, interpreter bytecode
   // handlers and optimized code).

src/execution/messages.cc

@@ -1302,12 +1302,13 @@ MessageTemplate UpdateErrorTemplate(CallPrinter::ErrorHint hint,
     case CallPrinter::ErrorHint::kNone:
       return default_id;
   }
+  return default_id;
 }
 
 }  // namespace
 
-Handle<JSObject> ErrorUtils::NewIteratorError(Isolate* isolate,
-                                              Handle<Object> source) {
+Handle<Object> ErrorUtils::NewIteratorError(Isolate* isolate,
+                                            Handle<Object> source) {
   MessageLocation location;
   CallPrinter::ErrorHint hint = CallPrinter::kNone;
   Handle<String> callsite = RenderCallSite(isolate, source, &location, &hint);
@@ -1351,13 +1352,13 @@ Object ErrorUtils::ThrowSpreadArgError(Isolate* isolate, MessageTemplate id,
     }
   }
 
-  isolate->ThrowAt(isolate->factory()->NewTypeError(id, callsite, object),
-                   &location);
-  return ReadOnlyRoots(isolate).exception();
+  Handle<Object> exception =
+      isolate->factory()->NewTypeError(id, callsite, object);
+  return isolate->Throw(*exception, &location);
 }
 
-Handle<JSObject> ErrorUtils::NewCalledNonCallableError(Isolate* isolate,
-                                                       Handle<Object> source) {
+Handle<Object> ErrorUtils::NewCalledNonCallableError(Isolate* isolate,
+                                                     Handle<Object> source) {
   MessageLocation location;
   CallPrinter::ErrorHint hint = CallPrinter::kNone;
   Handle<String> callsite = RenderCallSite(isolate, source, &location, &hint);
@@ -1366,7 +1367,7 @@ Handle<JSObject> ErrorUtils::NewCalledNonCallableError(Isolate* isolate,
   return isolate->factory()->NewTypeError(id, callsite);
 }
 
-Handle<JSObject> ErrorUtils::NewConstructedNonConstructable(
+Handle<Object> ErrorUtils::NewConstructedNonConstructable(
     Isolate* isolate, Handle<Object> source) {
   MessageLocation location;
   CallPrinter::ErrorHint hint = CallPrinter::kNone;
@@ -1375,6 +1376,10 @@ Handle<JSObject> ErrorUtils::NewConstructedNonConstructable(
   return isolate->factory()->NewTypeError(id, callsite);
 }
 
+Object ErrorUtils::ThrowLoadFromNullOrUndefined(Isolate* isolate,
+                                                Handle<Object> object) {
+  return ThrowLoadFromNullOrUndefined(isolate, object, MaybeHandle<Object>());
+}
 Object ErrorUtils::ThrowLoadFromNullOrUndefined(Isolate* isolate,
                                                 Handle<Object> object,
                                                 MaybeHandle<Object> key) {
@@ -1447,7 +1452,7 @@ Object ErrorUtils::ThrowLoadFromNullOrUndefined(Isolate* isolate,
     callsite = BuildDefaultCallSite(isolate, object);
   }
 
-  Handle<JSObject> error;
+  Handle<Object> error;
   Handle<String> property_name;
   if (is_destructuring) {
     if (maybe_property_name.ToHandle(&property_name)) {
@@ -1471,12 +1476,7 @@ Object ErrorUtils::ThrowLoadFromNullOrUndefined(Isolate* isolate,
     }
   }
 
-  if (location_computed) {
-    isolate->ThrowAt(error, &location);
-  } else {
-    isolate->Throw(*error);
-  }
-  return ReadOnlyRoots(isolate).exception();
+  return isolate->Throw(*error, location_computed ? &location : nullptr);
 }
 
 }  // namespace internal

src/execution/messages.h

@@ -297,16 +297,16 @@ class ErrorUtils : public AllStatic {
                                               Handle<JSObject> error,
                                               Handle<Object> stack_trace);
 
-  static Handle<JSObject> NewIteratorError(Isolate* isolate,
-                                           Handle<Object> source);
-  static Handle<JSObject> NewCalledNonCallableError(Isolate* isolate,
-                                                    Handle<Object> source);
-  static Handle<JSObject> NewConstructedNonConstructable(Isolate* isolate,
-                                                         Handle<Object> source);
-  // Returns the Exception sentinel.
+  static Handle<Object> NewIteratorError(Isolate* isolate,
+                                         Handle<Object> source);
+  static Handle<Object> NewCalledNonCallableError(Isolate* isolate,
+                                                  Handle<Object> source);
+  static Handle<Object> NewConstructedNonConstructable(Isolate* isolate,
+                                                       Handle<Object> source);
   static Object ThrowSpreadArgError(Isolate* isolate, MessageTemplate id,
                                     Handle<Object> object);
-  // Returns the Exception sentinel.
+  static Object ThrowLoadFromNullOrUndefined(Isolate* isolate,
+                                             Handle<Object> object);
   static Object ThrowLoadFromNullOrUndefined(Isolate* isolate,
                                              Handle<Object> object,
                                              MaybeHandle<Object> key);

src/json/json-parser.cc

@@ -273,7 +273,8 @@ void JsonParser<Char>::ReportUnexpectedToken(JsonToken token) {
   // separated source file.
   isolate()->debug()->OnCompileError(script);
   MessageLocation location(script, pos, pos + 1);
-  isolate()->ThrowAt(factory->NewSyntaxError(message, arg1, arg2), &location);
+  Handle<Object> error = factory->NewSyntaxError(message, arg1, arg2);
+  isolate()->Throw(*error, &location);
 
   // Move the cursor to the end so we won't be able to proceed parsing.
   cursor_ = end_;

src/objects/source-text-module.cc

@@ -189,7 +189,7 @@ MaybeHandle<Cell> SourceTextModule::ResolveExport(
     } else if (name_set->count(export_name)) {
       // Cycle detected.
       if (must_resolve) {
-        return isolate->ThrowAt<Cell>(
+        return isolate->Throw<Cell>(
             isolate->factory()->NewSyntaxError(
                 MessageTemplate::kCyclicModuleDependency, export_name,
                 module_specifier),
@@ -274,10 +274,10 @@ MaybeHandle<Cell> SourceTextModule::ResolveExportUsingStarExports(
               .ToHandle(&cell)) {
         if (unique_cell.is_null()) unique_cell = cell;
         if (*unique_cell != *cell) {
-          return isolate->ThrowAt<Cell>(isolate->factory()->NewSyntaxError(
-                                            MessageTemplate::kAmbiguousExport,
-                                            module_specifier, export_name),
-                                        &loc);
+          return isolate->Throw<Cell>(isolate->factory()->NewSyntaxError(
+                                          MessageTemplate::kAmbiguousExport,
+                                          module_specifier, export_name),
+                                      &loc);
         }
       } else if (isolate->has_pending_exception()) {
         return MaybeHandle<Cell>();
@@ -296,7 +296,7 @@ MaybeHandle<Cell> SourceTextModule::ResolveExportUsingStarExports(
 
   // Unresolvable.
   if (must_resolve) {
-    return isolate->ThrowAt<Cell>(
+    return isolate->Throw<Cell>(
         isolate->factory()->NewSyntaxError(MessageTemplate::kUnresolvableExport,
                                            module_specifier, export_name),
         &loc);

src/objects/synthetic-module.cc

@@ -59,7 +59,7 @@ MaybeHandle<Cell> SyntheticModule::ResolveExport(
 
   if (!must_resolve) return MaybeHandle<Cell>();
 
-  return isolate->ThrowAt<Cell>(
+  return isolate->Throw<Cell>(
       isolate->factory()->NewSyntaxError(MessageTemplate::kUnresolvableExport,
                                          module_specifier, export_name),
       &loc);

src/runtime/runtime-object.cc

@@ -1070,8 +1070,7 @@ RUNTIME_FUNCTION(Runtime_CopyDataPropertiesWithExcludedProperties) {
 
   // If source is undefined or null, throw a non-coercible error.
   if (source->IsNullOrUndefined(isolate)) {
-    return ErrorUtils::ThrowLoadFromNullOrUndefined(isolate, source,
-                                                    MaybeHandle<Object>());
+    return ErrorUtils::ThrowLoadFromNullOrUndefined(isolate, source);
   }
 
   ScopedVector<Handle<Object>> excluded_properties(args.length() - 1);