Skip to content

Commit

Permalink
Merge PR #199: Bug fixes, LSP, and debugger improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
@smarr
smarr committed Dec 10, 2017
2 parents 302f297 + 7f93924 commit 0f7a3e6
Show file tree
Hide file tree
Showing 22 changed files with 182 additions and 73 deletions.
49 changes: 25 additions & 24 deletions src/som/compiler/MethodBuilder.java
View file
Expand Up @@ -81,8 +81,8 @@ public final class MethodBuilder {
private boolean accessesVariablesOfOuterScope;
private boolean accessesLocalOfOuterScope;

private final LinkedHashMap<String, Argument> arguments = new LinkedHashMap<>();
private final LinkedHashMap<String, Local> locals = new LinkedHashMap<>();
private final LinkedHashMap<SSymbol, Argument> arguments = new LinkedHashMap<>();
private final LinkedHashMap<SSymbol, Local> locals = new LinkedHashMap<>();

private Internal frameOnStackVar;
private final MethodScope currentScope;
Expand Down Expand Up @@ -153,7 +153,7 @@ public void mergeIntoScope(final MethodScope scope, final SInvokable outer) {
for (Variable v : scope.getVariables()) {
Local l = v.splitToMergeIntoOuterScope(currentScope.getFrameDescriptor());
if (l != null) { // can happen for instance for the block self, which we omit
String name = l.getQualifiedName();
SSymbol name = l.getQualifiedName();
assert !locals.containsKey(name);
locals.put(name, l);
currentScope.addVariable(l);
Expand Down Expand Up @@ -196,7 +196,7 @@ public MixinScope getHolderScope() {

// Name for the frameOnStack slot,
// starting with ! to make it a name that's not possible in Smalltalk
private static final String FRAME_ON_STACK_SLOT_NAME = "!frameOnStack";
private static final SSymbol FRAME_ON_STACK_SLOT_NAME = Symbols.symbolFor("!frameOnStack");

public Internal getFrameOnStackMarkerVar() {
if (outerBuilder != null) {
Expand Down Expand Up @@ -344,8 +344,8 @@ public void setSignature(final SSymbol sig) {
signature = sig;
}

public void addArgument(final String arg, final SourceSection source) {
if (("self".equals(arg) || "$blockSelf".equals(arg)) && arguments.size() > 0) {
public void addArgument(final SSymbol arg, final SourceSection source) {
if ((Symbols.SELF == arg || Symbols.BLOCK_SELF == arg) && arguments.size() > 0) {
throw new IllegalStateException(
"The self argument always has to be the first argument of a method");
}
Expand All @@ -360,14 +360,14 @@ public void addArgument(final String arg, final SourceSection source) {

public Local addMessageCascadeTemp(final SourceSection source) throws MethodDefinitionError {
cascadeId += 1;
Local l = addLocal("$cascadeTmp" + cascadeId, true, source);
Local l = addLocal(Symbols.symbolFor("$cascadeTmp" + cascadeId), true, source);
if (currentScope.hasVariables()) {
currentScope.addVariable(l);
}
return l;
}

public Local addLocal(final String name, final boolean immutable,
public Local addLocal(final SSymbol name, final boolean immutable,
final SourceSection source) throws MethodDefinitionError {
if (arguments.containsKey(name)) {
throw new MethodDefinitionError("Method already defines argument " + name
Expand All @@ -389,7 +389,7 @@ public Local addLocal(final String name, final boolean immutable,
return l;
}

public Local addLocalAndUpdateScope(final String name, final boolean immutable,
public Local addLocalAndUpdateScope(final SSymbol name, final boolean immutable,
final SourceSection source) throws MethodDefinitionError {
Local l = addLocal(name, immutable, source);
currentScope.addVariable(l);
Expand Down Expand Up @@ -433,7 +433,7 @@ private int getOuterSelfContextLevel() {
* variable cannot be in scope and that a variable must have been erroneously
* by {@link #getReadNode} or {@link #getWriteNode}.
*/
private int getContextLevel(final String varName) {
private int getContextLevel(final SSymbol varName) {
// Check the current activation
if (locals.containsKey(varName) || arguments.containsKey(varName)) {
return 0;
Expand Down Expand Up @@ -462,7 +462,7 @@ public Local getEmbeddedLocal(final String embeddedName) {
* Refer to {@link #getContextLevel} for a description of how the
* enclosing scopes are traversed.
*/
protected Variable getVariable(final String varName) {
protected Variable getVariable(final SSymbol varName) {

// Check the current activation
if (locals.containsKey(varName)) {
Expand Down Expand Up @@ -501,7 +501,7 @@ protected Variable getVariable(final String varName) {
}

public Argument getSelf() {
return (Argument) getVariable("self");
return (Argument) getVariable(Symbols.SELF);
}

public ExpressionNode getSuperReadNode(@NotNull final SourceSection source) {
Expand All @@ -515,17 +515,17 @@ public ExpressionNode getSelfRead(@NotNull final SourceSection source) {
assert source != null;
MixinBuilder holder = getEnclosingMixinBuilder();
MixinDefinitionId mixinId = holder == null ? null : holder.getMixinId();
return getSelf().getSelfReadNode(getContextLevel("self"), mixinId, source);
return getSelf().getSelfReadNode(getContextLevel(Symbols.SELF), mixinId, source);
}

public ExpressionNode getReadNode(final String variableName,
public ExpressionNode getReadNode(final SSymbol variableName,
final SourceSection source) {
assert source != null;
Variable variable = getVariable(variableName);
return variable.getReadNode(getContextLevel(variableName), source);
}

public ExpressionNode getWriteNode(final String variableName,
public ExpressionNode getWriteNode(final SSymbol variableName,
final ExpressionNode valExpr, final SourceSection source) {
Local variable = getLocal(variableName);
return variable.getWriteNode(getContextLevel(variableName), valExpr, source);
Expand All @@ -534,17 +534,17 @@ public ExpressionNode getWriteNode(final String variableName,
public ExpressionNode getImplicitReceiverSend(final SSymbol selector,
final SourceSection source) {
// we need to handle super and self special here
if ("super".equals(selector.getString())) {
if (Symbols.SUPER == selector) {
return getSuperReadNode(source);
}
if ("self".equals(selector.getString())) {
if (Symbols.SELF == selector) {
return getSelfRead(source);
}

// first look up local and argument variables
Variable variable = getVariable(selector.getString());
Variable variable = getVariable(selector);
if (variable != null) {
return getReadNode(selector.getString(), source);
return getReadNode(selector, source);
}

// send the message to self if at top level (classes only) or
Expand All @@ -556,9 +556,9 @@ public ExpressionNode getImplicitReceiverSend(final SSymbol selector,
}

// then lookup non local and argument variables
Variable literalVariable = getVariable(selector.getString());
Variable literalVariable = getVariable(selector);
if (literalVariable != null) {
return getReadNode(selector.getString(), source);
return getReadNode(selector, source);
}

// otherwise it's an implicit send
Expand All @@ -573,7 +573,8 @@ public ExpressionNode getSetterSend(final SSymbol setter,
// write directly to local variables (excluding arguments)
String setterSend = setter.getString();
String setterName = setterSend.substring(0, setterSend.length() - 1);
String varName = setterName.substring(0, setterName.length() - 1);
String varNameStr = setterName.substring(0, setterName.length() - 1);
SSymbol varName = Symbols.symbolFor(varNameStr);

if (hasArgument(varName)) {
throw new MethodDefinitionError("Can't assign to argument: " + varName, source);
Expand Down Expand Up @@ -610,7 +611,7 @@ public ExpressionNode getSetterSend(final SSymbol setter,
source, language.getVM());
}

protected boolean hasArgument(final String varName) {
protected boolean hasArgument(final SSymbol varName) {
if (arguments.containsKey(varName)) {
return true;
}
Expand All @@ -625,7 +626,7 @@ protected boolean hasArgument(final String varName) {
* Refer to {@link #getContextLevel} for a description of how the
* enclosing scopes are traversed.
*/
protected Local getLocal(final String varName) {
protected Local getLocal(final SSymbol varName) {
// Check the current activation
if (locals.containsKey(varName)) {
return locals.get(varName);
Expand Down
4 changes: 2 additions & 2 deletions src/som/compiler/MixinBuilder.java
View file
Expand Up @@ -486,7 +486,7 @@ private MethodBuilder createSuperclassResolutionBuilder() {
}

// self is going to be the enclosing object
definitionMethod.addArgument("self",
definitionMethod.addArgument(Symbols.SELF,
SomLanguage.getSyntheticSource("self read", "super-class-resolution")
.createSection(1));
definitionMethod.setSignature(Symbols.DEF_CLASS);
Expand Down Expand Up @@ -582,7 +582,7 @@ protected List<ExpressionNode> createPrimaryFactoryArgumentRead(
args.add(objectInstantiationExpr);

for (Argument arg : arguments) {
if (!"self".equals(arg.name)) { // already have self as the newly instantiated object
if (Symbols.SELF != arg.name) { // already have self as the newly instantiated object
args.add(primaryFactoryMethod.getReadNode(arg.name, arg.source));
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/som/compiler/MixinDefinition.java
View file
Expand Up @@ -132,6 +132,10 @@ public SSymbol getName() {
return name;
}

public SourceSection getNameSourceSection() {
return nameSection;
}

/**
* Used by the SOMns Language Server.
*/
Expand Down Expand Up @@ -719,7 +723,7 @@ public void addSyntheticInitializerWithoutSuperSendOnlyForThingClass() {
SSymbol init = MixinBuilder.getInitializerName(Symbols.NEW);
MethodBuilder builder = new MethodBuilder(true, initializerBuilder.getLanguage(), null);
builder.setSignature(init);
builder.addArgument("self",
builder.addArgument(Symbols.SELF,
SomLanguage.getSyntheticSource("self read", "super-class-resolution")
.createSection(1));

Expand Down
20 changes: 10 additions & 10 deletions src/som/compiler/Parser.java
View file
Expand Up @@ -341,7 +341,7 @@ private MixinBuilder classDeclaration(final MixinBuilder outerBuilder,
messagePattern(primaryFactory);
} else {
// in the standard case, the primary factory method is #new
primaryFactory.addArgument("self", getEmptySource());
primaryFactory.addArgument(Symbols.SELF, getEmptySource());
primaryFactory.setSignature(Symbols.NEW);
}
mxnBuilder.setupInitializerBasedOnPrimaryFactory(getSource(coord));
Expand Down Expand Up @@ -855,7 +855,7 @@ private void methodDeclaration(final AccessModifier accessModifier,
}

private void messagePattern(final MethodBuilder builder) throws ParseError {
builder.addArgument("self", getEmptySource());
builder.addArgument(Symbols.SELF, getEmptySource());
switch (sym) {
case Identifier:
unaryPattern(builder);
Expand Down Expand Up @@ -885,7 +885,7 @@ protected void binaryPattern(final MethodBuilder builder) throws ParseError {
builder.addMethodDefinitionSource(getSource(coord));

coord = getCoordinate();
builder.addArgument(argument(), getSource(coord));
builder.addArgument(symbolFor(argument()), getSource(coord));
}

protected void keywordPattern(final MethodBuilder builder) throws ParseError {
Expand All @@ -896,7 +896,7 @@ protected void keywordPattern(final MethodBuilder builder) throws ParseError {
builder.addMethodDefinitionSource(getSource(coord));

coord = getCoordinate();
builder.addArgument(argument(), getSource(coord));
builder.addArgument(symbolFor(argument()), getSource(coord));
} while (sym == Keyword);

builder.setSignature(symbolFor(kw.toString()));
Expand Down Expand Up @@ -1027,7 +1027,7 @@ private void localDefinition(final MethodBuilder builder,
initializer = null;
}

Local local = builder.addLocal(slotName, immutable, source);
Local local = builder.addLocal(symbolFor(slotName), immutable, source);

if (initializer != null) {
SourceSection write = getSource(coord);
Expand Down Expand Up @@ -1563,7 +1563,7 @@ private Local getLoopIdx(final MethodBuilder builder,
// if it is a literal, we still need a memory location for counting, so,
// add a synthetic local
loopIdx = builder.addLocalAndUpdateScope(
"!i" + SourceCoordinate.getLocationQualifier(source), false, source);
symbolFor("!i" + SourceCoordinate.getLocationQualifier(source)), false, source);
}
return loopIdx;
}
Expand Down Expand Up @@ -1742,7 +1742,7 @@ private ExpressionNode literalObject(final MethodBuilder builder)

// Setup the builder and "new" factory for the implicit class
MethodBuilder primaryFactory = classBuilder.getPrimaryFactoryMethodBuilder();
primaryFactory.addArgument("self", getEmptySource());
primaryFactory.addArgument(Symbols.SELF, getEmptySource());
primaryFactory.setSignature(Symbols.NEW);
classBuilder.setupInitializerBasedOnPrimaryFactory(source);

Expand All @@ -1758,7 +1758,7 @@ private ExpressionNode literalObject(final MethodBuilder builder)
return new ObjectLiteralNode(mixinDef, outerRead, newMessage).initialize(source);
}

private SSymbol selector() throws ParseError {
protected SSymbol selector() throws ParseError {
if (sym == OperatorSequence || symIn(singleOpSyms)) {
return binarySelector();
} else if (sym == Keyword || sym == KeywordSequence) {
Expand Down Expand Up @@ -1796,7 +1796,7 @@ private ExpressionNode nestedBlock(final MethodBuilder builder)
SourceCoordinate coord = getCoordinate();
expect(NewBlock, DelimiterOpeningTag.class);

builder.addArgument("$blockSelf", getEmptySource());
builder.addArgument(Symbols.BLOCK_SELF, getEmptySource());

if (sym == Colon) {
blockPattern(builder);
Expand Down Expand Up @@ -1830,7 +1830,7 @@ private void blockArguments(final MethodBuilder builder) throws ParseError {
do {
expect(Colon, KeywordTag.class);
SourceCoordinate coord = getCoordinate();
builder.addArgument(argument(), getSource(coord));
builder.addArgument(symbolFor(argument()), getSource(coord));
} while (sym == Colon);
}

Expand Down

0 comments on commit 0f7a3e6

Please sign in to comment.