[parser] Treat var initializers in masking catch as assigning

This changes the existing implementation that creates an unresolved reference for those cases to look at exactly what scopes are relevant so it can correctly handle catch scopes and avoid re-resolving later.

Variable through with aren't marked as assigning since this information isn't relevant for the with itself; and if the with is passed through, there's no need to mark the outer variable as assigned since it's either initialized or it isn't.

The catch variable is assigned since it is relevant for the catch variable.

The CL uses LookupLocal which wouldn't work for deserialized scopes, but this isn't relevant because 1) eval scopes are declaration scopes, and 2) eval causes all outer variables to be maybe_assigned anyway.

Bug: chromium:1074737
Change-Id: I3febca479ddd1f3c62eae299190b06c0b4cd3746
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2187272
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67683}

src/parsing/expression-scope.h

@@ -53,15 +53,36 @@ class ExpressionScope {
       AsExpressionParsingScope()->TrackVariable(result);
     } else {
       Variable* var = Declare(name, pos);
-      if (IsVarDeclaration() && !parser()->scope()->is_declaration_scope()) {
-        // Make sure we'll properly resolve the variable since we might be in a
-        // with or catch scope. In those cases the proxy isn't guaranteed to
-        // refer to the declared variable, so consider it unresolved.
-        parser()->scope()->AddUnresolved(result);
-      } else {
-        DCHECK_NOT_NULL(var);
-        result->BindTo(var);
+      if (IsVarDeclaration()) {
+        bool passed_through_with = false;
+        for (Scope* scope = parser()->scope(); !scope->is_declaration_scope();
+             scope = scope->outer_scope()) {
+          if (scope->is_with_scope()) {
+            passed_through_with = true;
+          } else if (scope->is_catch_scope()) {
+            Variable* var = scope->LookupLocal(name);
+            // If a variable is declared in a catch scope with a masking
+            // catch-declared variable, the initializing assignment is an
+            // assignment to the catch-declared variable instead.
+            // https://tc39.es/ecma262/#sec-variablestatements-in-catch-blocks
+            if (var != nullptr) {
+              result->set_is_assigned();
+              if (passed_through_with) break;
+              result->BindTo(var);
+              var->SetMaybeAssigned();
+              return result;
+            }
+          }
+        }
+        if (passed_through_with) {
+          // If a variable is declared in a with scope, the initializing
+          // assignment might target a with-declared variable instead.
+          parser()->scope()->AddUnresolved(result);
+          return result;
+        }
       }
+      DCHECK_NOT_NULL(var);
+      result->BindTo(var);
     }
     return result;
   }

test/mjsunit/regress/regress-crbug-1074737.js

+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+try {
+  throw 42
+} catch (e) {
+  function foo() { return e };
+  %PrepareFunctionForOptimization(foo);
+  %OptimizeFunctionOnNextCall(foo);
+  foo();
+  var e = "expected";
+}
+assertEquals("expected", foo());
+
+try {
+  throw 42
+} catch (f) {
+  function foo2() { return f };
+  %PrepareFunctionForOptimization(foo2);
+  %OptimizeFunctionOnNextCall(foo2);
+  foo2();
+  with ({}) {
+    var f = "expected";
+  }
+}
+assertEquals("expected", foo2());
+
+(function () {
+  function foo3() { return g };
+  %PrepareFunctionForOptimization(foo3);
+  %OptimizeFunctionOnNextCall(foo3);
+  foo3();
+  with ({}) {
+    var g = "expected";
+  }
+  assertEquals("expected", foo3());
+})()