Handle method/mixin scope uniformly

Until now, we kept mixin and method scopes separately, which does not directly represent the lexical structure and can cause confusion. With this change, scope is managed as a single chain outwards, with methods and mixins on the same chain. This change applies to LexicalScope classes as well as to the Builder classes. - add ScopeBuilder superclass for MixinBuilder and MethodBuilder - correctly determine whether scopes are immutable also for methods - added Local.isMutable() - InliningVisitor, handle case that there are no variables Signed-off-by: Stefan Marr <[email protected]>
smarr · Mar 19, 2018 · 1f94eca · 1f94eca
1 parent 1d86f8c
commit 1f94eca
Show file tree

Hide file tree

Showing 11 changed files with 364 additions and 267 deletions.
diff --git a/src/som/compiler/MethodBuilder.java b/src/som/compiler/MethodBuilder.java
diff --git a/src/som/compiler/MixinBuilder.java b/src/som/compiler/MixinBuilder.java
@@ -38,7 +38,9 @@
 import som.compiler.MixinDefinition.SlotDefinition;
 import som.compiler.MixinDefinition.SlotMutator;
 import som.compiler.Variable.Argument;
-import som.interpreter.LexicalScope.MethodScope;
+import som.compiler.Variable.Internal;
+import som.compiler.Variable.Local;
+import som.interpreter.LexicalScope;
 import som.interpreter.LexicalScope.MixinScope;
 import som.interpreter.Method;
 import som.interpreter.SNodeFactory;
@@ -58,7 +60,7 @@
  * MixinBuilders are used by the parser to accumulate all information to create
  * a {@link MixinDefinition}.
  */
-public final class MixinBuilder {
+public final class MixinBuilder extends ScopeBuilder<MixinScope> {
   // TODO: if performance critical, optimize mixin builder by initializing structures lazily
 
   /** The method that is used to resolve the superclass at runtime. */
@@ -103,12 +105,8 @@ public final class MixinBuilder {
 
   private final AccessModifier accessModifier;
 
-  private final MixinScope instanceScope;
   private final MixinScope classScope;
 
-  private final MixinBuilder  outerMixin;
-  private final MethodBuilder outerMethod;
-
   private final MixinDefinitionId mixinId;
 
   private final StructuralProbe structuralProbe;
@@ -138,66 +136,28 @@ public String toString() {
     }
   };
 
-  private MixinBuilder(final MixinBuilder outerMixin, final MethodBuilder outerMethod,
-      final AccessModifier accessModifier, final SSymbol name,
-      final SourceSection nameSection,
-      final StructuralProbe structuralProbe,
-      final SomLanguage language) {
+  public MixinBuilder(final ScopeBuilder<?> outer, final AccessModifier accessModifier,
+      final SSymbol name, final SourceSection nameSection,
+      final StructuralProbe structuralProbe, final SomLanguage language) {
+    super(outer, outer == null ? null : outer.getScope());
     this.name = name;
     this.nameSection = nameSection;
     this.mixinId = new MixinDefinitionId(name);
 
     this.classSide = false;
-    this.outerMixin = outerMixin;
-    this.outerMethod = outerMethod;
     this.language = language;
 
-    // classes can only be defined on the instance side,
-    // so, both time the instance scope
-    MethodScope outerMethodScope =
-        outerMethod != null ? outerMethod.getCurrentMethodScope() : null;
-    MixinScope outerMixinScope = outerMixin != null ? outerMixin.getInstanceScope() : null;
-    this.instanceScope = new MixinScope(outerMixinScope, outerMethodScope);
-    this.classScope = new MixinScope(outerMixinScope, outerMethodScope);
+    this.classScope = createScope(scope);
 
-    this.initializer = new MethodBuilder(this, this.instanceScope, structuralProbe);
-    this.primaryFactoryMethod = new MethodBuilder(this, this.classScope, structuralProbe);
+    this.initializer = new MethodBuilder(this, structuralProbe);
+    this.primaryFactoryMethod =
+        new MethodBuilder(this, classScope, false, language, structuralProbe);
     this.superclassAndMixinResolutionBuilder = createSuperclassResolutionBuilder();
 
     this.accessModifier = accessModifier;
     this.structuralProbe = structuralProbe;
   }
 
-  /**
-   * This constructor is used to create a MixinBuilder for a Newspeak classes,
-   * which must either occur at the top lexical level or be nested inside of
-   * another class declaration (the <code>outerMixin</code>).
-   */
-  public MixinBuilder(final MixinBuilder outerMixin,
-      final AccessModifier accessModifier, final SSymbol name,
-      final SourceSection nameSection,
-      final StructuralProbe structuralProbe,
-      final SomLanguage language) {
-    this(outerMixin, null, accessModifier, name, nameSection,
-        structuralProbe, language);
-  }
-
-  /**
-   * This constructor is used to create a MixinBuilder for an object literal,
-   * which must be inside of an activation (the <code>outerMethod</code>).
-   * Since the object literal inherits from a class, we still need to hold
-   * a reference to the outer class declaration - the mixin that
-   * encloses the current activation.
-   */
-  public MixinBuilder(final MethodBuilder outerMethod,
-      final AccessModifier accessModifier, final SSymbol name,
-      final SourceSection nameSection,
-      final StructuralProbe structuralProbe,
-      final SomLanguage language) {
-    this(outerMethod.getEnclosingMixinBuilder(), outerMethod,
-        accessModifier, name, nameSection, structuralProbe, language);
-  }
-
   public static class MixinDefinitionError extends SemanticDefinitionError {
     private static final long serialVersionUID = 5030639383869198851L;
 
@@ -206,38 +166,82 @@ public static class MixinDefinitionError extends SemanticDefinitionError {
     }
   }
 
-  public SomLanguage getLanguage() {
-    return language;
+  @Override
+  protected MixinScope createScope(final LexicalScope outer) {
+    return new MixinScope(outer);
   }
 
-  public MixinScope getInstanceScope() {
-    return instanceScope;
+  @Override
+  public MixinBuilder getMixin() {
+    return this;
   }
 
-  public MixinBuilder getOuterBuilder() {
-    return outerMixin;
+  @Override
+  public MethodBuilder getMethod() {
+    return null;
   }
 
-  public boolean isLiteral() {
-    return outerMethod != null;
+  public SomLanguage getLanguage() {
+    return language;
   }
 
-  public MethodBuilder getEnclosingMethod() {
-    return outerMethod;
+  public boolean isLiteral() {
+    return outer instanceof MethodBuilder;
   }
 
-  public SSymbol getName() {
-    return name;
+  @Override
+  public String getName() {
+    return name.getString();
   }
 
   public boolean isModule() {
-    return outerMixin == null;
+    return outer == null;
   }
 
   public AccessModifier getAccessModifier() {
     return accessModifier;
   }
 
+  @Override
+  protected int getContextLevel(final SSymbol varName) {
+    assert outer != null : "If there is no outer context, "
+        + "something is wrong with lexcial scoping. Could not find var: "
+        + varName.getString();
+    return outer.getContextLevel(varName);
+  }
+
+  @Override
+  protected Local getLocal(final SSymbol varName) {
+    if (outer == null) {
+      return null;
+    }
+    return outer.getLocal(varName);
+  }
+
+  @Override
+  protected Variable getVariable(final SSymbol varName) {
+    if (outer == null) {
+      return null;
+    }
+    return outer.getVariable(varName);
+  }
+
+  @Override
+  protected boolean hasArgument(final SSymbol varName) {
+    if (outer == null) {
+      return false;
+    }
+    return outer.hasArgument(varName);
+  }
+
+  @Override
+  public Internal getFrameOnStackMarkerVar() {
+    // null, because we use this for non-local returns,
+    // which are returning from methods
+    // so, with this method, we just look for the closest enclosing object method
+    return null;
+  }
+
   /**
    * Expression to resolve the super class at runtime, used in the instantiation.
    */
@@ -409,7 +413,7 @@ public MixinScope getScopeForCurrentParserPosition() {
     if (classSide) {
       return classScope;
     } else {
-      return instanceScope;
+      return scope;
     }
   }
 
@@ -442,9 +446,9 @@ public MixinDefinition assemble(final SourceSection source) {
         primaryFactory.getSignature(), slotAndInitExprs, initializer,
         initializerSource, superclassResolution,
         slots, dispatchables, factoryMethods, embeddedMixins, mixinId,
-        accessModifier, instanceScope, classScope, allSlotsAreImmutable,
-        outerScopeIsImmutable(), isModule(), source);
-    instanceScope.setMixinDefinition(clsDef, false);
+        accessModifier, scope, classScope, allSlotsAreImmutable,
+        isModule() || outer.isImmutable(), isModule(), source);
+    scope.setMixinDefinition(clsDef, false);
     classScope.setMixinDefinition(clsDef, true);
 
     setHolders(clsDef);
@@ -455,11 +459,17 @@ public MixinDefinition assemble(final SourceSection source) {
     return clsDef;
   }
 
-  private boolean outerScopeIsImmutable() {
-    if (isModule()) {
-      return true;
+  @Override
+  protected boolean isImmutable() {
+    if (!allSlotsAreImmutable) {
+      return false;
     }
-    return outerMixin.allSlotsAreImmutable && outerMixin.outerScopeIsImmutable();
+
+    if (outer != null) {
+      return outer.isImmutable();
+    }
+
+    return true;
   }
 
   private void setHolders(final MixinDefinition clsDef) {
@@ -480,8 +490,8 @@ private MethodBuilder createSuperclassResolutionBuilder() {
     if (isModule()) {
       definitionMethod = new MethodBuilder(true, language, structuralProbe);
     } else {
-      definitionMethod = new MethodBuilder(outerMixin,
-          outerMixin.getInstanceScope(), structuralProbe);
+      definitionMethod =
+          new MethodBuilder(outer, outer.scope, false, language, structuralProbe);
     }
 
     // self is going to be the enclosing object

diff --git a/src/som/compiler/MixinDefinition.java b/src/som/compiler/MixinDefinition.java
@@ -796,9 +796,8 @@ private void adaptInvokableDispatchables(final MethodScope scope, final int appl
 
   public MixinDefinition cloneAndAdaptAfterScopeChange(final MethodScope adaptedScope,
       final int appliesTo) {
-    MixinScope adaptedInstanceScope =
-        new MixinScope(instanceScope.getOuterMixin(), adaptedScope);
-    MixinScope adaptedClassScope = new MixinScope(classScope.getOuterMixin(), adaptedScope);
+    MixinScope adaptedInstanceScope = new MixinScope(adaptedScope);
+    MixinScope adaptedClassScope = new MixinScope(adaptedScope);
 
     MixinDefinition clone = cloneForSplitting(adaptedInstanceScope, adaptedClassScope);
 

diff --git a/src/som/compiler/Parser.java b/src/som/compiler/Parser.java
@@ -829,8 +829,7 @@ public SourceSection getSource(final SourceCoordinate coord) {
   private void methodDeclaration(final AccessModifier accessModifier,
       final SourceCoordinate coord, final MixinBuilder mxnBuilder)
       throws ProgramDefinitionError {
-    MethodBuilder builder = new MethodBuilder(
-        mxnBuilder, mxnBuilder.getScopeForCurrentParserPosition(), structuralProbe);
+    MethodBuilder builder = new MethodBuilder(mxnBuilder, structuralProbe);
 
     comments();
 
@@ -1441,8 +1440,8 @@ protected ExpressionNode keywordMessage(final MethodBuilder builder,
     } else {
       assert !eventualSend;
       return createImplicitReceiverSend(msg, args,
-          builder.getCurrentMethodScope(),
-          builder.getEnclosingMixinBuilder().getMixinId(), source, language.getVM());
+          builder.getScope(),
+          builder.getMixin().getMixinId(), source, language.getVM());
     }
   }
 
@@ -1690,7 +1689,7 @@ private ExpressionNode nestedBlock(final MethodBuilder builder)
       blockPattern(builder);
     }
 
-    String outerMethodName = stripColons(builder.getOuterBuilder().getSignature().getString());
+    String outerMethodName = stripColons(builder.getOuter().getName());
 
     // generate Block signature
     String blockSig = "λ" + outerMethodName + "@" + coord.startLine + "@" + coord.startColumn;

diff --git a/src/som/compiler/ScopeBuilder.java b/src/som/compiler/ScopeBuilder.java
@@ -0,0 +1,50 @@
+package som.compiler;
+
+import som.compiler.Variable.Internal;
+import som.compiler.Variable.Local;
+import som.interpreter.LexicalScope;
+import som.vmobjects.SSymbol;
+
+
+/**
+ * The super class of {@link MixinBuilder} and {@link MethodBuilder}.
+ * It manages scope information;
+ */
+public abstract class ScopeBuilder<LS extends LexicalScope> {
+  protected final ScopeBuilder<? extends LexicalScope> outer;
+
+  protected final LS scope;
+
+  protected ScopeBuilder(final ScopeBuilder<?> outer, final LexicalScope outerScope) {
+    this.outer = outer;
+    this.scope = createScope(outerScope);
+  }
+
+  public final ScopeBuilder<?> getOuter() {
+    return outer;
+  }
+
+  public LS getScope() {
+    return scope;
+  }
+
+  protected abstract LS createScope(LexicalScope outer);
+
+  public abstract MixinBuilder getMixin();
+
+  public abstract MethodBuilder getMethod();
+
+  protected abstract int getContextLevel(SSymbol varName);
+
+  protected abstract Local getLocal(SSymbol varName);
+
+  protected abstract Variable getVariable(SSymbol varName);
+
+  protected abstract boolean hasArgument(SSymbol varName);
+
+  public abstract Internal getFrameOnStackMarkerVar();
+
+  protected abstract boolean isImmutable();
+
+  public abstract String getName();
+}
diff --git a/src/som/compiler/Variable.java b/src/som/compiler/Variable.java
@@ -209,6 +209,8 @@ public FrameSlot getSlot() {
 
     protected abstract Local create();
 
+    public abstract boolean isMutable();
+
     @Override
     public Local split(final FrameDescriptor descriptor) {
       Local newLocal = create();
@@ -252,6 +254,11 @@ public static final class MutableLocal extends Local {
     public Local create() {
       return new MutableLocal(name, source);
     }
+
+    @Override
+    public boolean isMutable() {
+      return true;
+    }
   }
 
   public static final class ImmutableLocal extends Local {
@@ -263,6 +270,11 @@ public static final class ImmutableLocal extends Local {
     public Local create() {
       return new ImmutableLocal(name, source);
     }
+
+    @Override
+    public boolean isMutable() {
+      return false;
+    }
   }
 
   /**