Adding support for const redeclaration on REPL mode.

This change adds support for `const` redeclaration on REPL mode with
the semantincs recommended in the design doc:

1) REPL scripts should not be able to reassign bindings to `const`
   variables.

2) Re-declaring `const` variables of page scripts is not allowed in
   REPL scripts.

3) Re-declearing `const` variables is not allowed in the same REPL
   script.

4) `const` re-declaration is allowed across separate REPL scripts.

5) Old references to previously declared variables get updated with the
   new value, even those references from within optimized functions.

Design doc: https://goo.gle/devtools-const-repl

Bug: chromium:1076427
Change-Id: Ic73d2ae7fcfbfc1f5b58f61e0c3c69e9c4d85d77
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2865721
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Commit-Queue: Luis Fernando Pardo Sixtos <lpardosixtos@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#74510}

src/ast/variables.cc

@@ -30,14 +30,15 @@ bool Variable::IsGlobalObjectProperty() const {
          scope_ != nullptr && scope_->is_script_scope();
 }
 
-bool Variable::IsReplGlobalLet() const {
-  return scope()->is_repl_mode_scope() && mode() == VariableMode::kLet;
+bool Variable::IsReplGlobal() const {
+  return scope()->is_repl_mode_scope() &&
+         (mode() == VariableMode::kLet || mode() == VariableMode::kConst);
 }
 
 void Variable::RewriteLocationForRepl() {
   DCHECK(scope_->is_repl_mode_scope());
 
-  if (mode() == VariableMode::kLet) {
+  if (mode() == VariableMode::kLet || mode() == VariableMode::kConst) {
     DCHECK_EQ(location(), VariableLocation::CONTEXT);
     bit_field_ =
         LocationField::update(bit_field_, VariableLocation::REPL_GLOBAL);

src/ast/variables.h

@@ -125,8 +125,9 @@ class Variable final : public ZoneObject {
   bool IsLookupSlot() const { return location() == VariableLocation::LOOKUP; }
   bool IsGlobalObjectProperty() const;
 
-  // True for 'let' variables declared in the script scope of a REPL script.
-  bool IsReplGlobalLet() const;
+  // True for 'let' and 'const' variables declared in the script scope of a REPL
+  // script.
+  bool IsReplGlobal() const;
 
   bool is_dynamic() const { return IsDynamicVariableMode(mode()); }
 

src/execution/execution.cc

@@ -190,8 +190,12 @@ MaybeHandle<Context> NewScriptContext(Isolate* isolate,
       if (IsLexicalVariableMode(mode) || IsLexicalVariableMode(lookup.mode)) {
         Handle<Context> context = ScriptContextTable::GetContext(
             isolate, script_context, lookup.context_index);
-        // If we are trying to re-declare a REPL-mode let as a let, allow it.
-        if (!(mode == VariableMode::kLet && lookup.mode == VariableMode::kLet &&
+        // If we are trying to re-declare a REPL-mode let as a let or REPL-mode
+        // const as a const, allow it.
+        if (!(((mode == VariableMode::kLet &&
+                lookup.mode == VariableMode::kLet) ||
+               (mode == VariableMode::kConst &&
+                lookup.mode == VariableMode::kConst)) &&
               scope_info->IsReplModeScope() &&
               context->scope_info().IsReplModeScope())) {
           // ES#sec-globaldeclarationinstantiation 5.b:

src/ic/ic.cc

@@ -522,9 +522,12 @@ MaybeHandle<Object> LoadGlobalIC::Load(Handle<Name> name,
 
       bool use_ic = (state() != NO_FEEDBACK) && FLAG_use_ic && update_feedback;
       if (use_ic) {
+        // 'const' Variables are mutable if REPL mode is enabled. This disables
+        // compiler inlining for all 'const' variables declared in REPL mode.
         if (nexus()->ConfigureLexicalVarMode(
                 lookup_result.context_index, lookup_result.slot_index,
-                lookup_result.mode == VariableMode::kConst)) {
+                (lookup_result.mode == VariableMode::kConst &&
+                 !lookup_result.is_repl_mode))) {
           TRACE_HANDLER_STATS(isolate(), LoadGlobalIC_LoadScriptContextField);
         } else {
           // Given combination of indices can't be encoded, so use slow stub.

src/interpreter/bytecode-generator.cc

@@ -3289,7 +3289,7 @@ void BytecodeGenerator::BuildVariableLoad(Variable* variable,
       break;
     }
     case VariableLocation::REPL_GLOBAL: {
-      DCHECK(variable->IsReplGlobalLet());
+      DCHECK(variable->IsReplGlobal());
       FeedbackSlot slot = GetCachedLoadGlobalICSlot(typeof_mode, variable);
       builder()->LoadGlobal(variable->raw_name(), feedback_index(slot),
                             typeof_mode);
@@ -3478,7 +3478,8 @@ void BytecodeGenerator::BuildVariableAssignment(
       break;
     }
     case VariableLocation::REPL_GLOBAL: {
-      // A let declaration like 'let x = 7' is effectively translated to:
+      // A let or const declaration like 'let x = 7' is effectively translated
+      // to:
       //   <top of the script>:
       //     ScriptContext.x = TheHole;
       //   ...
@@ -3488,19 +3489,23 @@ void BytecodeGenerator::BuildVariableAssignment(
       // The ScriptContext slot for 'x' that we store to here is not
       // necessarily the ScriptContext of this script, but rather the
       // first ScriptContext that has a slot for name 'x'.
-      DCHECK(variable->IsReplGlobalLet());
+      DCHECK(variable->IsReplGlobal());
       if (op == Token::INIT) {
         RegisterList store_args = register_allocator()->NewRegisterList(2);
         builder()
             ->StoreAccumulatorInRegister(store_args[1])
             .LoadLiteral(variable->raw_name())
             .StoreAccumulatorInRegister(store_args[0]);
-        builder()->CallRuntime(Runtime::kStoreGlobalNoHoleCheckForReplLet,
-                               store_args);
+        builder()->CallRuntime(
+            Runtime::kStoreGlobalNoHoleCheckForReplLetOrConst, store_args);
       } else {
-        FeedbackSlot slot =
-            GetCachedStoreGlobalICSlot(language_mode(), variable);
-        builder()->StoreGlobal(variable->raw_name(), feedback_index(slot));
+        if (mode == VariableMode::kConst) {
+          builder()->CallRuntime(Runtime::kThrowConstAssignError);
+        } else {
+          FeedbackSlot slot =
+              GetCachedStoreGlobalICSlot(language_mode(), variable);
+          builder()->StoreGlobal(variable->raw_name(), feedback_index(slot));
+        }
       }
       break;
     }

src/objects/contexts.cc

@@ -55,6 +55,7 @@ bool ScriptContextTable::Lookup(Isolate* isolate, ScriptContextTable table,
   for (int i = 0; i < table.synchronized_used(); i++) {
     Context context = table.get_context(i);
     DCHECK(context.IsScriptContext());
+    result->is_repl_mode = context.scope_info().IsReplModeScope();
     int slot_index = ScopeInfo::ContextSlotIndex(
         context.scope_info(), name, &result->mode, &result->init_flag,
         &result->maybe_assigned_flag, &is_static_flag);

src/objects/contexts.h

@@ -358,6 +358,9 @@ class ScriptContextTable : public FixedArray {
   struct LookupResult {
     int context_index;
     int slot_index;
+    // repl_mode flag is needed to disable inlining of 'const' variables in REPL
+    // mode.
+    bool is_repl_mode;
     VariableMode mode;
     InitializationFlag init_flag;
     MaybeAssignedFlag maybe_assigned_flag;

src/objects/scope-info.h

@@ -213,8 +213,8 @@ class ScopeInfo : public TorqueGeneratedScopeInfo<ScopeInfo, HeapObject> {
   // closest outer class when resolving private names.
   bool PrivateNameLookupSkipsOuterClass() const;
 
-  // REPL mode scopes allow re-declaraction of let variables. They come from
-  // debug evaluate but are different to IsDebugEvaluateScope().
+  // REPL mode scopes allow re-declaraction of let and const variables. They
+  // come from debug evaluate but are different to IsDebugEvaluateScope().
   bool IsReplModeScope() const;
 
 #ifdef DEBUG

src/runtime/runtime-scopes.cc

@@ -859,7 +859,7 @@ RUNTIME_FUNCTION(Runtime_StoreLookupSlot_SloppyHoisting) {
                                LanguageMode::kSloppy, lookup_flags));
 }
 
-RUNTIME_FUNCTION(Runtime_StoreGlobalNoHoleCheckForReplLet) {
+RUNTIME_FUNCTION(Runtime_StoreGlobalNoHoleCheckForReplLetOrConst) {
   HandleScope scope(isolate);
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_HANDLE_CHECKED(String, name, 0);

src/runtime/runtime.h

@@ -410,28 +410,28 @@ namespace internal {
   F(StringReplaceNonGlobalRegExpWithFunction, 3, 1)              \
   F(StringSplit, 3, 1)
 
-#define FOR_EACH_INTRINSIC_SCOPES(F, I)     \
-  F(DeclareEvalFunction, 2, 1)              \
-  F(DeclareEvalVar, 1, 1)                   \
-  F(DeclareGlobals, 2, 1)                   \
-  F(DeclareModuleExports, 2, 1)             \
-  F(DeleteLookupSlot, 1, 1)                 \
-  F(LoadLookupSlot, 1, 1)                   \
-  F(LoadLookupSlotInsideTypeof, 1, 1)       \
-                                            \
-  F(NewClosure, 2, 1)                       \
-  F(NewClosure_Tenured, 2, 1)               \
-  F(NewFunctionContext, 1, 1)               \
-  F(NewRestParameter, 1, 1)                 \
-  F(NewSloppyArguments, 1, 1)               \
-  F(NewStrictArguments, 1, 1)               \
-  F(PushBlockContext, 1, 1)                 \
-  F(PushCatchContext, 2, 1)                 \
-  F(PushWithContext, 2, 1)                  \
-  F(StoreGlobalNoHoleCheckForReplLet, 2, 1) \
-  F(StoreLookupSlot_Sloppy, 2, 1)           \
-  F(StoreLookupSlot_SloppyHoisting, 2, 1)   \
-  F(StoreLookupSlot_Strict, 2, 1)           \
+#define FOR_EACH_INTRINSIC_SCOPES(F, I)            \
+  F(DeclareEvalFunction, 2, 1)                     \
+  F(DeclareEvalVar, 1, 1)                          \
+  F(DeclareGlobals, 2, 1)                          \
+  F(DeclareModuleExports, 2, 1)                    \
+  F(DeleteLookupSlot, 1, 1)                        \
+  F(LoadLookupSlot, 1, 1)                          \
+  F(LoadLookupSlotInsideTypeof, 1, 1)              \
+                                                   \
+  F(NewClosure, 2, 1)                              \
+  F(NewClosure_Tenured, 2, 1)                      \
+  F(NewFunctionContext, 1, 1)                      \
+  F(NewRestParameter, 1, 1)                        \
+  F(NewSloppyArguments, 1, 1)                      \
+  F(NewStrictArguments, 1, 1)                      \
+  F(PushBlockContext, 1, 1)                        \
+  F(PushCatchContext, 2, 1)                        \
+  F(PushWithContext, 2, 1)                         \
+  F(StoreGlobalNoHoleCheckForReplLetOrConst, 2, 1) \
+  F(StoreLookupSlot_Sloppy, 2, 1)                  \
+  F(StoreLookupSlot_SloppyHoisting, 2, 1)          \
+  F(StoreLookupSlot_Strict, 2, 1)                  \
   F(ThrowConstAssignError, 0, 1)
 
 #define FOR_EACH_INTRINSIC_STRINGS(F, I)  \

test/debugger/debug/debug-evaluate-repl-mode-optimized.js

@@ -30,3 +30,36 @@ assertEquals(21, foo());
 // script context.
 evaluate('x; let x;');
 assertEquals(undefined, foo());
+
+// Test that a const declared variable 'y' bound by an optimized function is
+// updated properly.
+evaluate('const y = 42;');
+
+evaluate('function foo1() { return y; }');
+
+%PrepareFunctionForOptimization(foo1);
+foo1();
+foo1();
+%OptimizeFunctionOnNextCall(foo1);
+assertEquals(42, foo1());
+assertOptimized(foo1);
+
+evaluate('const y = 21');
+assertEquals(21, foo1());
+
+// Test that a const declared variable 'z' bound by an optimized function is
+// updated properly.
+evaluate('const z = {a:0};');
+
+evaluate('function foo2() { z.a = 42; }');
+
+%PrepareFunctionForOptimization(foo2);
+foo2();
+foo2();
+%OptimizeFunctionOnNextCall(foo2);
+foo2();
+assertOptimized(foo2);
+
+evaluate('const z = {a:21}');
+foo2();
+assertEquals(42, z.a);

test/debugger/debug/debug-evaluate-repl-mode.js

@@ -103,42 +103,85 @@ assertDoesNotThrow(() => result = evaluate("class K {};"));
 
 // result = evaluate("toString;");
 
-// Re-declare let as const
-evaluate("let z = 10;");
-assertThrows(() => result = evaluate("const z = 9;"),
-    SyntaxError, "Identifier 'z' has already been declared");
-result = await evaluate("z;");
-assertEquals(10, result);
+// Declare const and get value
+result = await evaluate("const c = 7;");
+result = await evaluate("c;");
+assertEquals(7, result);
+
+// Re-declare in the same script after declaration in another script.
+assertThrows(() => evaluate("let c = 8; let c = 9;"));
+result = await evaluate("c;");
+assertEquals(7, result);
 
 // Re-declare const as const
+result = await evaluate("const c = 8;");
+result = await evaluate("c;");
+assertEquals(8, result);
+
+// Assign to const
+assertThrowsAsync(evaluate("c = 11;"),
+    TypeError, "Assignment to constant variable.");
+result = await evaluate("c;");
+assertEquals(8, result);
+
+await evaluate("const c = 8;");
+
+// Close over const. Inner function is only pre-parsed.
+result = await evaluate("function getter() { return c; }");
+assertEquals(undefined, result);
+result = await evaluate("getter();");
+assertEquals(8, result);
+// Modifies the original c; does not create a new one/shadow.
 result = await evaluate("const c = 10;");
-assertThrows(() => result = evaluate("const c = 11;"),
-    SyntaxError, "Identifier 'c' has already been declared");
+assertEquals(undefined, result);
 result = await evaluate("c;");
 assertEquals(10, result);
+result = await evaluate("getter();");
+assertEquals(10, result);
+
+await evaluate("const c = 10");
+
+// Check store from an inner scope throws error.
+assertThrowsAsync(evaluate("{ let z; c = 11; };"),
+    TypeError, "Assignment to constant variable.");
+result = await evaluate("c;");
+assertEquals(10, result);
+
+// Check re-declare from an inner scope does nothing.
+result = await evaluate("{ let z; const c = 12; } c;");
+assertEquals(10, result);
+
+assertThrowsAsync(evaluate("{ const qq = 10; } qq;"),
+    ReferenceError, "qq is not defined");
 
 // Const vs. const in same script.
 assertThrows(() => result = evaluate("const d = 9; const d = 10;"),
 SyntaxError, "Identifier 'd' has already been declared");
 
-// Close over const
-result = await evaluate("const e = 10; function closure() { return e; }");
-result = await evaluate("e;");
-assertEquals(10, result);
+// Check TDZ; const use before initialization.
+assertThrowsAsync(evaluate("e; const e = 7;"), ReferenceError);
+assertThrowsAsync(evaluate("e;"), ReferenceError);
 
-// Assign to const
-assertThrowsAsync(evaluate("e = 11;"),
-    TypeError, "Assignment to constant variable.");
-result = await evaluate("e;");
-assertEquals(10, result);
-result = await evaluate("closure();");
-assertEquals(10, result);
+result = await evaluate("const e = 8;");
+assertEquals(undefined, result);
+result = await evaluate("e;")
+assertEquals(8, result);
 
-// Assign to const in TDZ
-assertThrowsAsync(evaluate("f; const f = 11;"),
-    ReferenceError, "Cannot access 'f' before initialization");
-assertThrowsAsync(evaluate("f = 12;"),
-    TypeError, "Assignment to constant variable.");
+// f is marked as constant in TDZ
+assertThrowsAsync(evaluate("f = 10; const f = 7;"),
+  TypeError, ("Assignment to constant variable."));
+result = await evaluate("const f = 11;");
+assertEquals(undefined, result);
+// We fixed 'f'!
+result = await evaluate("f;");
+assertEquals(11, result);
+
+// Re-declare let as const
+evaluate("let z = 10;");
+assertThrows(() => result = evaluate("const z = 9;"),
+    SyntaxError, "Identifier 'z' has already been declared");
+result = await evaluate("z;");
+assertEquals(10, result)
 
 // Re-declare const as let
 result = await evaluate("const g = 12;");
@@ -246,6 +289,53 @@ assertEquals(2, result);
 result = await evaluate("typeof k11");
 assertEquals("undefined", result);
 
+// Non-configurable properties of the global object (also created by plain old
+// top-level var declarations) cannot be re-declared as const.
+result = await evaluate(`Object.defineProperty(globalThis, 'k12', {
+  value: 1,
+  configurable: false
+});`);
+result = await evaluate("k12;");
+assertEquals(1, result);
+assertThrows(() => result = evaluate("const k12 = 2;"),
+    SyntaxError, "Identifier 'k12' has already been declared");
+result = await evaluate("k12;");
+assertEquals(1, result);
+
+// ... Except if you do it in the same script.
+result = await evaluate(`Object.defineProperty(globalThis, 'k13', {
+  value: 1,
+  configurable: false
+});
+const k13 = 2;`);
+result = await evaluate("k13;");
+assertEquals(2, result);
+result = await evaluate("globalThis.k13;");
+assertEquals(1, result);
+
+// But if the property is configurable then both versions are allowed.
+result = await evaluate(`Object.defineProperty(globalThis, 'k14', {
+  value: 1,
+  configurable: true
+});`);
+result = await evaluate("k14;");
+assertEquals(1, result);
+result = await evaluate("const k14 = 2;");
+result = await evaluate("k14;");
+assertEquals(2, result);
+result = await evaluate("globalThis.k14;");
+assertEquals(1, result);
+
+result = await evaluate(`Object.defineProperty(globalThis, 'k15', {
+  value: 1,
+  configurable: true
+});
+const k15 = 2;`);
+result = await evaluate("k15;");
+assertEquals(2, result);
+result = await evaluate("globalThis.k15;");
+assertEquals(1, result);
+
 // Test lets with names on the object prototype e.g. toString to make sure
 // it only works for own properties.
 // result = evaluate("let valueOf;");
@@ -307,6 +397,18 @@ evaluate("let l10 = 42; function fn2() { return l10; }");
 evaluate("let l10 = 'foo';");
 assertEquals("foo", fn2());
 
+// Check that binding and re-declaring a function via const works.
+result = evaluate("const fn3 = function() { return 21; }");
+assertEquals(21, fn3());
+result = evaluate("const fn3 = function() { return 42; }");
+assertEquals(42, fn3());
+
+// Check that lazily parsed functions that bind a REPL-const variable work.
+evaluate("const l11 = 21;");
+evaluate("const l11 = 42; function fn4() { return l11; }");
+evaluate("const l11 = 'foo1';");
+assertEquals("foo1", fn4());
+
 })().catch(e => {
     print(e);
     print(e.stack);