From 4bfc062aaf1fc94e48525578dbf886caf8561988 Mon Sep 17 00:00:00 2001 From: TopchetoEU <36534413+TopchetoEU@users.noreply.github.com> Date: Thu, 5 Sep 2024 17:13:34 +0300 Subject: [PATCH] fix: correctly flatten locals in control flow statements --- .../jscript/compilation/CompoundNode.java | 34 +++++++++---- .../compilation/FunctionArrowNode.java | 2 +- .../jscript/compilation/FunctionNode.java | 2 +- .../jscript/compilation/JavaScript.java | 2 +- .../compilation/VariableDeclareNode.java | 2 +- .../compilation/control/ForInNode.java | 3 +- .../jscript/compilation/control/ForNode.java | 7 +-- .../compilation/control/ForOfNode.java | 3 +- .../compilation/control/WhileNode.java | 3 +- .../compilation/scope/FunctionScope.java | 7 +-- .../jscript/compilation/scope/Scope.java | 49 +++++++++++++------ .../compilation/scope/VariableList.java | 14 +++++- .../compilation/values/VariableNode.java | 6 +-- 13 files changed, 92 insertions(+), 42 deletions(-) diff --git a/src/main/java/me/topchetoeu/jscript/compilation/CompoundNode.java b/src/main/java/me/topchetoeu/jscript/compilation/CompoundNode.java index f89ebd6..29888f6 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/CompoundNode.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/CompoundNode.java @@ -13,17 +13,21 @@ import me.topchetoeu.jscript.common.parsing.Source; public class CompoundNode extends Node { public final Node[] statements; + public final boolean hasScope; public Location end; @Override public void resolve(CompileResult target) { for (var stm : statements) stm.resolve(target); } - public void compile(CompileResult target, boolean pollute, boolean alloc, BreakpointType type) { + public void compile(CompileResult target, boolean pollute, boolean singleEntry, BreakpointType type) { List statements = new ArrayList(); - var subtarget = alloc ? target.subtarget() : target; - if (alloc) subtarget.add(i -> Instruction.stackAlloc(subtarget.scope.allocCount())); + var subtarget = hasScope ? target.subtarget() : target; + if (hasScope) { + subtarget.add(i -> Instruction.stackAlloc(subtarget.scope.allocCount())); + subtarget.scope.singleEntry = singleEntry; + } for (var stm : this.statements) { if (stm instanceof FunctionStatementNode func) { @@ -41,7 +45,7 @@ public class CompoundNode extends Node { else stm.compile(subtarget, polluted = pollute, BreakpointType.STEP_OVER); } - if (alloc) { + if (hasScope) { subtarget.scope.end(); subtarget.add(_i -> Instruction.stackFree(subtarget.scope.allocCount())); } @@ -60,11 +64,21 @@ public class CompoundNode extends Node { return this; } - public CompoundNode(Location loc, Node ...statements) { + public CompoundNode(Location loc, boolean hasScope, Node ...statements) { super(loc); + this.hasScope = hasScope; this.statements = statements; } + public static void compileMultiEntry(Node node, CompileResult target, boolean pollute, BreakpointType type) { + if (node instanceof CompoundNode comp) { + comp.compile(target, pollute, false, type); + } + else { + node.compile(target, pollute, type); + } + } + public static ParseRes parseComma(Source src, int i, Node prev, int precedence) { if (precedence > 1) return ParseRes.failed(); @@ -78,14 +92,14 @@ public class CompoundNode extends Node { if (!curr.isSuccess()) return curr.chainError(src.loc(i + n), "Expected a value after the comma"); n += curr.n; - if (prev instanceof CompoundNode) { + if (prev instanceof CompoundNode comp) { var children = new ArrayList(); - children.addAll(List.of(((CompoundNode)prev).statements)); + children.addAll(List.of(comp.statements)); children.add(curr.result); - return ParseRes.res(new CompoundNode(loc, children.toArray(Node[]::new)), n); + return ParseRes.res(new CompoundNode(loc, comp.hasScope, children.toArray(Node[]::new)), n); } - else return ParseRes.res(new CompoundNode(loc, prev, curr.result), n); + else return ParseRes.res(new CompoundNode(loc, false, prev, curr.result), n); } public static ParseRes parse(Source src, int i) { var n = Parsing.skipEmpty(src, i); @@ -115,6 +129,6 @@ public class CompoundNode extends Node { statements.add(res.result); } - return ParseRes.res(new CompoundNode(loc, statements.toArray(Node[]::new)).setEnd(src.loc(i + n - 1)), n); + return ParseRes.res(new CompoundNode(loc, true, statements.toArray(Node[]::new)).setEnd(src.loc(i + n - 1)), n); } } diff --git a/src/main/java/me/topchetoeu/jscript/compilation/FunctionArrowNode.java b/src/main/java/me/topchetoeu/jscript/compilation/FunctionArrowNode.java index f816f3f..036533c 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/FunctionArrowNode.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/FunctionArrowNode.java @@ -24,7 +24,7 @@ public class FunctionArrowNode extends FunctionNode { private static final CompoundNode expToBody(Node node) { if (node instanceof CompoundNode res) return res; - else return new CompoundNode(node.loc(), new ReturnNode(node.loc(), node)); + else return new CompoundNode(node.loc(), false, new ReturnNode(node.loc(), node)); } public static ParseRes parse(Source src, int i) { diff --git a/src/main/java/me/topchetoeu/jscript/compilation/FunctionNode.java b/src/main/java/me/topchetoeu/jscript/compilation/FunctionNode.java index ea13605..154b360 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/FunctionNode.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/FunctionNode.java @@ -84,7 +84,7 @@ public abstract class FunctionNode extends Node { } body.resolve(target); - body.compile(target, lastReturn, false, BreakpointType.NONE); + body.compile(target, lastReturn, BreakpointType.NONE); scope.end(); diff --git a/src/main/java/me/topchetoeu/jscript/compilation/JavaScript.java b/src/main/java/me/topchetoeu/jscript/compilation/JavaScript.java index 94dfcd2..5e06b59 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/JavaScript.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/JavaScript.java @@ -323,7 +323,7 @@ public final class JavaScript { } public static CompileResult compile(Environment env, Node ...statements) { - var func = new FunctionValueNode(null, null, new Parameters(List.of()), new CompoundNode(null, statements), null); + var func = new FunctionValueNode(null, null, new Parameters(List.of()), new CompoundNode(null, true, statements), null); var res = func.compileBody(env, new FunctionScope(true), true, null, null); res.buildTask.run(); return res; diff --git a/src/main/java/me/topchetoeu/jscript/compilation/VariableDeclareNode.java b/src/main/java/me/topchetoeu/jscript/compilation/VariableDeclareNode.java index 5fc137f..7584d48 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/VariableDeclareNode.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/VariableDeclareNode.java @@ -46,7 +46,7 @@ public class VariableDeclareNode extends Node { target.add(VariableNode.toSet(target, entry.location, entry.name, false, true)); } else target.add(_i -> { - var i = target.scope.get(entry.name, true); + var i = target.scope.get(entry.name, false); if (i == null) return Instruction.globDef(entry.name); else return Instruction.nop(); diff --git a/src/main/java/me/topchetoeu/jscript/compilation/control/ForInNode.java b/src/main/java/me/topchetoeu/jscript/compilation/control/ForInNode.java index 7c65d31..59688a5 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/control/ForInNode.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/control/ForInNode.java @@ -8,6 +8,7 @@ import me.topchetoeu.jscript.common.parsing.ParseRes; import me.topchetoeu.jscript.common.parsing.Parsing; import me.topchetoeu.jscript.common.parsing.Source; import me.topchetoeu.jscript.compilation.CompileResult; +import me.topchetoeu.jscript.compilation.CompoundNode; import me.topchetoeu.jscript.compilation.DeferredIntSupplier; import me.topchetoeu.jscript.compilation.JavaScript; import me.topchetoeu.jscript.compilation.LabelContext; @@ -47,7 +48,7 @@ public class ForInNode extends Node { var end = new DeferredIntSupplier(); LabelContext.pushLoop(target.env, loc(), label, end, start); - body.compile(target, false, BreakpointType.STEP_OVER); + CompoundNode.compileMultiEntry(body, target, false, BreakpointType.STEP_OVER); LabelContext.popLoop(target.env, label); int endI = target.size(); diff --git a/src/main/java/me/topchetoeu/jscript/compilation/control/ForNode.java b/src/main/java/me/topchetoeu/jscript/compilation/control/ForNode.java index f09148b..8e9651a 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/control/ForNode.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/control/ForNode.java @@ -7,6 +7,7 @@ import me.topchetoeu.jscript.common.parsing.ParseRes; import me.topchetoeu.jscript.common.parsing.Parsing; import me.topchetoeu.jscript.common.parsing.Source; import me.topchetoeu.jscript.compilation.CompileResult; +import me.topchetoeu.jscript.compilation.CompoundNode; import me.topchetoeu.jscript.compilation.DeferredIntSupplier; import me.topchetoeu.jscript.compilation.JavaScript; import me.topchetoeu.jscript.compilation.LabelContext; @@ -29,16 +30,16 @@ public class ForNode extends Node { declaration.compile(subtarget, false, BreakpointType.STEP_OVER); int start = subtarget.size(); - condition.compile(subtarget, true, BreakpointType.STEP_OVER); + CompoundNode.compileMultiEntry(condition, subtarget, true, BreakpointType.STEP_OVER); int mid = subtarget.temp(); var end = new DeferredIntSupplier(); LabelContext.pushLoop(subtarget.env, loc(), label, end, start); - body.compile(subtarget, false, BreakpointType.STEP_OVER); + CompoundNode.compileMultiEntry(body, subtarget, false, BreakpointType.STEP_OVER); LabelContext.popLoop(subtarget.env, label); - assignment.compile(subtarget, false, BreakpointType.STEP_OVER); + CompoundNode.compileMultiEntry(assignment, subtarget, false, BreakpointType.STEP_OVER); int endI = subtarget.size(); end.set(endI); diff --git a/src/main/java/me/topchetoeu/jscript/compilation/control/ForOfNode.java b/src/main/java/me/topchetoeu/jscript/compilation/control/ForOfNode.java index 9f55541..b675e7b 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/control/ForOfNode.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/control/ForOfNode.java @@ -7,6 +7,7 @@ import me.topchetoeu.jscript.common.parsing.ParseRes; import me.topchetoeu.jscript.common.parsing.Parsing; import me.topchetoeu.jscript.common.parsing.Source; import me.topchetoeu.jscript.compilation.CompileResult; +import me.topchetoeu.jscript.compilation.CompoundNode; import me.topchetoeu.jscript.compilation.DeferredIntSupplier; import me.topchetoeu.jscript.compilation.JavaScript; import me.topchetoeu.jscript.compilation.LabelContext; @@ -51,7 +52,7 @@ public class ForOfNode extends Node { var end = new DeferredIntSupplier(); LabelContext.pushLoop(target.env, loc(), label, end, start); - body.compile(target, false, BreakpointType.STEP_OVER); + CompoundNode.compileMultiEntry(body, target, false, BreakpointType.STEP_OVER); LabelContext.popLoop(target.env, label); int endI = target.size(); diff --git a/src/main/java/me/topchetoeu/jscript/compilation/control/WhileNode.java b/src/main/java/me/topchetoeu/jscript/compilation/control/WhileNode.java index 94330cf..d6d9447 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/control/WhileNode.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/control/WhileNode.java @@ -7,6 +7,7 @@ import me.topchetoeu.jscript.common.parsing.ParseRes; import me.topchetoeu.jscript.common.parsing.Parsing; import me.topchetoeu.jscript.common.parsing.Source; import me.topchetoeu.jscript.compilation.CompileResult; +import me.topchetoeu.jscript.compilation.CompoundNode; import me.topchetoeu.jscript.compilation.DeferredIntSupplier; import me.topchetoeu.jscript.compilation.JavaScript; import me.topchetoeu.jscript.compilation.LabelContext; @@ -28,7 +29,7 @@ public class WhileNode extends Node { LabelContext.pushLoop(target.env, loc(), label, end, start); - body.compile(target, false, BreakpointType.STEP_OVER); + CompoundNode.compileMultiEntry(body, target, false, BreakpointType.STEP_OVER); LabelContext.popLoop(target.env, label); var endI = target.size(); diff --git a/src/main/java/me/topchetoeu/jscript/compilation/scope/FunctionScope.java b/src/main/java/me/topchetoeu/jscript/compilation/scope/FunctionScope.java index 757c4d8..4f8f143 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/scope/FunctionScope.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/scope/FunctionScope.java @@ -60,9 +60,10 @@ public class FunctionScope extends Scope { var superRes = super.get(name, capture); if (superRes != null) return superRes; - if (specials.has(name)) return specials.get(name); - if (locals.has(name)) return locals.get(name); - if (captures.has(name)) return captures.get(name); + if (specials.has(name)) return addCaptured(specials.get(name), capture); + if (locals.has(name)) return addCaptured(locals.get(name), capture); + if (captures.has(name)) return addCaptured(captures.get(name), capture); + if (captureParent == null) return null; var parentVar = captureParent.get(name, true); diff --git a/src/main/java/me/topchetoeu/jscript/compilation/scope/Scope.java b/src/main/java/me/topchetoeu/jscript/compilation/scope/Scope.java index 7846efd..3129e36 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/scope/Scope.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/scope/Scope.java @@ -18,6 +18,11 @@ public class Scope { public final Scope parent; public final HashSet captured = new HashSet<>(); + protected final Variable addCaptured(Variable var, boolean captured) { + if (captured) this.captured.add(var); + return var; + } + /** * Wether or not the scope is going to be entered multiple times. * If set to true, captured variables will be kept as allocations, otherwise will be converted to locals @@ -76,7 +81,7 @@ public class Scope { */ public Variable get(String name, boolean capture) { var res = variables.get(name); - if (res != null) return res; + if (res != null) return addCaptured(res, capture); if (parent != null) return parent.get(name, capture); return null; @@ -107,8 +112,14 @@ public class Scope { * @return Whether or not the request was actually fuliflled */ public boolean flattenVariable(Variable variable, boolean capturable) { - if (parent == null) return false; - return parent.flattenVariable(variable, capturable); + if (singleEntry || !capturable) { + if (parent == null) return false; + return parent.flattenVariable(variable, capturable); + } + else { + variables.overlay(variable); + return true; + } } public int localsCount() { return 0; } @@ -123,15 +134,6 @@ public class Scope { this.ended = true; - for (var v : variables.all()) { - if (captured.contains(v)) { - if (singleEntry) this.flattenVariable(v, true); - } - else { - this.flattenVariable(v, false); - } - } - if (this.parent != null) { assert this.parent.child == this; this.parent.child = null; @@ -146,13 +148,30 @@ public class Scope { */ public boolean finish() { if (finished) return false; - if (parent != null && !parent.finished) throw new IllegalStateException("Tried to finish a child before the parent was finished"); + if (parent != null && parent.finished) throw new IllegalStateException("Tried to finish a child after the parent was finished"); + + for (var child : prevChildren) child.finish(); + + var captured = new HashSet(); + var normal = new HashSet(); + + for (var v : variables.all()) { + if (this.captured.contains(v)) { + if (singleEntry) captured.add(v); + } + else normal.add(v); + } + + for (var v : captured) variables.remove(v); + for (var v : normal) variables.remove(v); + + for (var v : captured) flattenVariable(v, true); + for (var v : normal) flattenVariable(v, false); + this.variables.freeze(); this.finished = true; - for (var child : prevChildren) child.finish(); - return true; } diff --git a/src/main/java/me/topchetoeu/jscript/compilation/scope/VariableList.java b/src/main/java/me/topchetoeu/jscript/compilation/scope/VariableList.java index c67859d..9dfed4b 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/scope/VariableList.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/scope/VariableList.java @@ -61,6 +61,7 @@ public final class VariableList { private final HashMap map = new HashMap<>(); private ArrayList frozenList = null; + private HashMap varMap = new HashMap<>(); private final IntSupplier offset; private IntUnaryOperator indexConverter = null; @@ -105,6 +106,7 @@ public final class VariableList { } map.put(val.name, node); + varMap.put(val, node); val.setIndexSupplier(node); return val; @@ -117,9 +119,15 @@ public final class VariableList { return this.add(val, true); } public Variable remove(String key) { + var res = map.get(key); + if (res != null) return remove(res.var); + else return null; + } + public Variable remove(Variable var) { + if (var == null) return null; if (frozen()) throw new RuntimeException("The scope has been frozen"); - var node = map.get(key); + var node = varMap.get(var); if (node == null) return null; if (node.prev != null) { @@ -143,6 +151,9 @@ public final class VariableList { node.next = null; node.prev = null; + map.remove(node.var.name); + varMap.remove(node.var); + return node.var; } @@ -178,6 +189,7 @@ public final class VariableList { } first = last = null; + varMap = null; } public Iterable all() { diff --git a/src/main/java/me/topchetoeu/jscript/compilation/values/VariableNode.java b/src/main/java/me/topchetoeu/jscript/compilation/values/VariableNode.java index 02c4d36..0bf8ea6 100644 --- a/src/main/java/me/topchetoeu/jscript/compilation/values/VariableNode.java +++ b/src/main/java/me/topchetoeu/jscript/compilation/values/VariableNode.java @@ -24,7 +24,7 @@ public class VariableNode extends Node implements AssignableNode { } @Override public void compile(CompileResult target, boolean pollute) { - var i = target.scope.get(name, true); + var i = target.scope.get(name, false); if (i == null) { target.add(_i -> { @@ -40,7 +40,7 @@ public class VariableNode extends Node implements AssignableNode { } public static IntFunction toGet(CompileResult target, Location loc, String name, Supplier onGlobal) { - var i = target.scope.get(name, true); + var i = target.scope.get(name, false); if (i == null) return _i -> { if (target.scope.has(name, false)) return Instruction.throwSyntax(loc, String.format("Cannot access '%s' before initialization", name)); @@ -54,7 +54,7 @@ public class VariableNode extends Node implements AssignableNode { public static IntFunction toSet(CompileResult target, Location loc, String name, boolean keep, boolean define) { - var i = target.scope.get(name, true); + var i = target.scope.get(name, false); if (i == null) return _i -> { if (target.scope.has(name, false)) return Instruction.throwSyntax(loc, String.format("Cannot access '%s' before initialization", name));