Repository navigation
Conversation
A reference to a binding from inside its own declaration was always
treated as a TDZ access, so `const { a, b = a } = o` (and the same with
let, var and parameter patterns) threw a ReferenceError. Now the
reference is only in the TDZ if it comes before the binding, or if it is
in the initializer or default value that the binding gets destructured
from.
Block function declarations were instantiated in source order, before the block's lexical environment was entered for a later escaping let or const. The function then captured the outer environment and could not see the binding. Instantiate function declarations after the other declarations of the block.
A private getter/setter pair shares one Private Name but was counted twice, so static private fields after it got an offset past the end of the class's Private Environment and the engine panicked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the first three bugs from #1002. The fourth one (
pop/shift/spliceon a non-extensible array) touches the Array internals, so I'll send it as a separate PR.One commit per bug:
Destructuring default reading an earlier binding (
bytecode_compiler.rs).IdentifierReference::compiletreated any reference whose ancestor is the binding's ownVariableDeclarator/FormalParameteras a TDZ access, soconst { a, b = a } = { a: 1 }threw. The newis_declaration_tdzkeeps the TDZ only when the reference comes before the binding, or sits in the initializer / default value that the binding gets destructured from. So these still throw:const { a = a } = {},const [a] = [a],const [{ a } = { a: a }] = [],const { [a]: a } = {},(x = x) => x. And these now work:const { a, b = a } = o,let [a, b = a] = arr,({ width, height = width }) => ...,const [[a, b = a] = [7]] = [].Block function can't see a later
let/const(block_declaration_instantiation.rs). Functions were instantiated in source order, before the block's lexical environment was entered for the later escaping binding, so the closure captured the outer environment. Function declarations are now instantiated after the other declarations of the block. The order isn't observable otherwise: the other declarations only create bindings.Panic with a static private field and a private getter/setter pair (
class_definition_evaluation.rs).get #bandset #bshare one Private Name, butinstance_private_method_countcounted both, so the static field offset went pastprivate_env.names.len(). The method counters now only go up for a new name.Testing
nova_vm/tests/language_tests.rsruns three new files intests/sources/(the repros from the issue plus the TDZ cases above). All three fail onmain(twoReferenceErrors and thePrivateNamepanic) and pass with this branch. I checked every case against Node first.cargo test,cargo fmt --checkandcargo clippy --all-targets -- -D warningspass.-u. No result changed apart from a few timeouts that pass on my machine (I left those out), soexpectations.jsonis unchanged.