Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix semantics of empty blocks #19

Merged
merged 7 commits into from
Aug 29, 2020
Merged

Fix semantics of empty blocks #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2a01d68
Added uninitialized locals test
smarr Aug 24, 2020
be21b36
Added tests for uninitialized blocks
smarr Aug 24, 2020
4dc173d
Replace use of Vector with ArrayList
smarr Aug 24, 2020
23da37e
Fix formatting
smarr Aug 24, 2020
16d08aa
Add Parser.toString
smarr Aug 24, 2020
7b2697d
Make MGenc a little less mutable
smarr Aug 24, 2020
ea9f341
Make sure the empty blocks return nil
smarr Aug 24, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core-lib
Open in desktop
Submodule core-lib updated 4 files
+15 −0 TestSuite/BasicInterpreterTests/Blocks.som
+1 −1 TestSuite/BasicInterpreterTests/NumberOfTests.som
+12 −0 TestSuite/BasicInterpreterTests/Regressions.som
+2 −0 TestSuite/IntegerTest.som
64 changes: 44 additions & 20 deletions src/som/compiler/MethodGenerationContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,25 @@

package som.compiler;

import static som.interpreter.Bytecodes.*;
import static som.interpreter.Bytecodes.DUP;
import static som.interpreter.Bytecodes.HALT;
import static som.interpreter.Bytecodes.POP;
import static som.interpreter.Bytecodes.POP_ARGUMENT;
import static som.interpreter.Bytecodes.POP_FIELD;
import static som.interpreter.Bytecodes.POP_LOCAL;
import static som.interpreter.Bytecodes.PUSH_ARGUMENT;
import static som.interpreter.Bytecodes.PUSH_BLOCK;
import static som.interpreter.Bytecodes.PUSH_CONSTANT;
import static som.interpreter.Bytecodes.PUSH_FIELD;
import static som.interpreter.Bytecodes.PUSH_GLOBAL;
import static som.interpreter.Bytecodes.PUSH_LOCAL;
import static som.interpreter.Bytecodes.RETURN_LOCAL;
import static som.interpreter.Bytecodes.RETURN_NON_LOCAL;
import static som.interpreter.Bytecodes.SEND;
import static som.interpreter.Bytecodes.SUPER_SEND;

import java.util.ArrayList;
import java.util.List;
import java.util.Vector;

import som.vm.Universe;
import som.vmobjects.SAbstractObject;
Expand All @@ -41,19 +55,33 @@

public class MethodGenerationContext {

private ClassGenerationContext holderGenc;
private MethodGenerationContext outerGenc;
private boolean blockMethod;
private final ClassGenerationContext holderGenc;
private final MethodGenerationContext outerGenc;
private final boolean blockMethod;

private SSymbol signature;
private final List<String> arguments = new ArrayList<String>();
private boolean primitive;
private final List<String> locals = new ArrayList<String>();
private final List<SAbstractObject> literals = new ArrayList<SAbstractObject>();
private boolean finished;
private final Vector<Byte> bytecode = new Vector<Byte>();
private final ArrayList<Byte> bytecode = new ArrayList<>();

/**
* Constructor used for block methods.
*/
public MethodGenerationContext(final ClassGenerationContext holderGenc,
final MethodGenerationContext outerGenc) {
this.holderGenc = holderGenc;
this.outerGenc = outerGenc;
blockMethod = outerGenc != null;
}

public void setHolder(final ClassGenerationContext cgenc) {
holderGenc = cgenc;
/**
* Constructor used for normal methods.
*/
public MethodGenerationContext(final ClassGenerationContext holderGenc) {
this(holderGenc, null);
}

public void addArgument(final String arg) {
Expand Down Expand Up @@ -94,7 +122,7 @@ private int computeStackDepth() {
int i = 0;

while (i < bytecode.size()) {
switch (bytecode.elementAt(i)) {
switch (bytecode.get(i)) {
case HALT:
i++;
break;
Expand Down Expand Up @@ -131,7 +159,7 @@ private int computeStackDepth() {
case SUPER_SEND: {
// these are special: they need to look at the number of
// arguments (extractable from the signature)
SSymbol sig = (SSymbol) literals.get(bytecode.elementAt(i + 1));
SSymbol sig = (SSymbol) literals.get(bytecode.get(i + 1));

depth -= sig.getNumberOfSignatureArguments();

Expand All @@ -145,7 +173,7 @@ private int computeStackDepth() {
break;
default:
throw new IllegalStateException("Illegal bytecode "
+ bytecode.elementAt(i));
+ bytecode.get(i));
}

if (depth > maxDepth) {
Expand Down Expand Up @@ -194,8 +222,12 @@ public void addLocal(final String local) {
locals.add(local);
}

public boolean hasBytecodes() {
return !bytecode.isEmpty();
}

public void removeLastBytecode() {
bytecode.removeElementAt(bytecode.size() - 1);
bytecode.remove(bytecode.size() - 1);
}

public boolean isBlockMethod() {
Expand All @@ -215,18 +247,10 @@ public boolean addLiteralIfAbsent(final SAbstractObject lit) {
return true;
}

public void setIsBlockMethod(final boolean isBlock) {
blockMethod = isBlock;
}

public ClassGenerationContext getHolder() {
return holderGenc;
}

public void setOuter(final MethodGenerationContext mgenc) {
outerGenc = mgenc;
}

public byte addLiteral(final SAbstractObject lit) {
int i = literals.size();
assert i < 128;
Expand Down
29 changes: 20 additions & 9 deletions src/som/compiler/Parser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,11 @@ public Parser(final Reader reader, final Universe universe, final String filenam
getSymbolFromLexer();
}

@Override
public String toString() {
return filename + ":" + lexer.getCurrentLineNumber() + ":" + lexer.getCurrentColumn();
}

public void classdef(final ClassGenerationContext cgenc) throws ProgramDefinitionError {
cgenc.setName(universe.symbolFor(text));
expect(Identifier);
Expand All @@ -197,8 +202,7 @@ public void classdef(final ClassGenerationContext cgenc) throws ProgramDefinitio
instanceFields(cgenc);
while (sym == Identifier || sym == Keyword || sym == OperatorSequence
|| symIn(binaryOpSyms)) {
MethodGenerationContext mgenc = new MethodGenerationContext();
mgenc.setHolder(cgenc);
MethodGenerationContext mgenc = new MethodGenerationContext(cgenc);
mgenc.addArgument("self");

method(mgenc);
Expand All @@ -215,8 +219,7 @@ public void classdef(final ClassGenerationContext cgenc) throws ProgramDefinitio
classFields(cgenc);
while (sym == Identifier || sym == Keyword || sym == OperatorSequence
|| symIn(binaryOpSyms)) {
MethodGenerationContext mgenc = new MethodGenerationContext();
mgenc.setHolder(cgenc);
MethodGenerationContext mgenc = new MethodGenerationContext(cgenc);
mgenc.addArgument("self");

method(mgenc);
Expand Down Expand Up @@ -453,6 +456,12 @@ private void blockBody(final MethodGenerationContext mgenc, final boolean seenPe
// was terminated with a . or not)
mgenc.removeLastBytecode();
}
if (mgenc.isBlockMethod() && !mgenc.hasBytecodes()) {
// if the block is empty, we need to return nil
SSymbol nilSym = universe.symbolFor("nil");
mgenc.addLiteralIfAbsent(nilSym);
bcGen.emitPUSHGLOBAL(mgenc, nilSym);
}
bcGen.emitRETURNLOCAL(mgenc);
mgenc.setFinished();
} else if (sym == EndTerm) {
Expand Down Expand Up @@ -556,11 +565,7 @@ private boolean primary(final MethodGenerationContext mgenc)
nestedTerm(mgenc);
break;
case NewBlock: {
MethodGenerationContext bgenc = new MethodGenerationContext();
bgenc.setIsBlockMethod(true);
bgenc.setHolder(mgenc.getHolder());
bgenc.setOuter(mgenc);

MethodGenerationContext bgenc = new MethodGenerationContext(mgenc.getHolder(), mgenc);
nestedBlock(bgenc);

SMethod blockMethod = bgenc.assemble(universe);
Expand Down Expand Up @@ -879,6 +884,12 @@ private void nestedBlock(final MethodGenerationContext mgenc) throws ProgramDefi
// expression
// in the block was not terminated by ., and can generate a return
if (!mgenc.isFinished()) {
if (!mgenc.hasBytecodes()) {
// if the block is empty, we need to return nil
SSymbol nilSym = universe.symbolFor("nil");
mgenc.addLiteralIfAbsent(nilSym);
bcGen.emitPUSHGLOBAL(mgenc, nilSym);
}
bcGen.emitRETURNLOCAL(mgenc);
mgenc.setFinished(true);
}
Expand Down
7 changes: 6 additions & 1 deletion tests/som/tests/BasicInterpreterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ public static Iterable<Object[]> data() {
{"Blocks", "testArg2", 77, SInteger.class},
{"Blocks", "testArgAndLocal", 8, SInteger.class},
{"Blocks", "testArgAndContext", 8, SInteger.class},
{"Blocks", "testEmptyZeroArg", 1, SInteger.class},
{"Blocks", "testEmptyOneArg", 1, SInteger.class},
{"Blocks", "testEmptyTwoArg", 1, SInteger.class},

{"Return", "testReturnSelf", "Return", SClass.class},
{"Return", "testReturnSelfImplicitly", "Return", SClass.class},
Expand Down Expand Up @@ -113,10 +116,12 @@ public static Iterable<Object[]> data() {

{"Regressions", "testSymbolEquality", 1, SInteger.class},
{"Regressions", "testSymbolReferenceEquality", 1, SInteger.class},
{"Regressions", "testUninitializedLocal", 1, SInteger.class},
{"Regressions", "testUninitializedLocalInBlock", 1, SInteger.class},

{"BinaryOperation", "test", 3 + 8, SInteger.class},

{"NumberOfTests", "numberOfTests", 52, SInteger.class}
{"NumberOfTests", "numberOfTests", 57, SInteger.class}
});
}

Expand Down