Skip to content

Commit

Permalink
Somewhat cleaned up scope joiners. It is not perfect, but it will hav…
Browse files Browse the repository at this point in the history
…e to

do for now. #186
  • Loading branch information
jurcovicovam committed Jul 1, 2014
1 parent 28d4897 commit fdbffec
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 118 deletions.
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.github.sommeri.less4j.core.compiler.expressions;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Stack;

Expand Down Expand Up @@ -40,6 +39,7 @@
import com.github.sommeri.less4j.core.compiler.scopes.IScope;
import com.github.sommeri.less4j.core.compiler.scopes.NullScope;
import com.github.sommeri.less4j.core.compiler.scopes.ScopeFactory;
import com.github.sommeri.less4j.core.compiler.stages.CallerCalleeScopeJoiner;
import com.github.sommeri.less4j.core.problems.BugHappened;
import com.github.sommeri.less4j.core.problems.ProblemsHandler;
import com.github.sommeri.less4j.utils.CssPrinter;
Expand All @@ -51,6 +51,7 @@ public class ExpressionsEvaluator {
private final IScope lazyScope;
private final Stack<IScope> eagerScopes = new Stack<IScope>();
private final ProblemsHandler problemsHandler;
private final CallerCalleeScopeJoiner scopesJoiner = new CallerCalleeScopeJoiner();

private ArithmeticCalculator arithmeticCalculator;
private ColorsCalculator colorsCalculator;
Expand Down Expand Up @@ -153,11 +154,11 @@ private Expression evaluate(Variable input, boolean failOnUndefined) {
return handleUndefinedVariable(input, failOnUndefined);
}

IScope originalScope = enteringScopeOf(expression);
IScope addedScope = enteringScopeOf(expression);
cycleDetector.enteringVariableValue(input);
Expression result = evaluate(expression);
cycleDetector.leftVariableValue();
leavingScope(originalScope);
leavingScope(addedScope);
return result;
}

Expand Down Expand Up @@ -287,7 +288,6 @@ private boolean canCompareDimensions(Dimension left, Dimension right) {
}

public Expression evaluate(FunctionExpression input) {
// FIXME: !!!!!!!!!!! input parameter has null scope <- use closes parental scope if the current scope is null
Expression evaluatedParameter = evaluate(input.getParameter());
List<Expression> splitParameters = (evaluatedParameter.getType() == ASTCssNodeType.EMPTY_EXPRESSION) ? new ArrayList<Expression>() : evaluatedParameter.splitByComma();

Expand Down Expand Up @@ -418,36 +418,25 @@ public Expression evaluate(DetachedRuleset input) {
return clone;
}

//FIXME !!!!!!! do something smarter, if they are local to each other they need special handling
//FIXME !!!!!!! make sure detached ruleset does not tie its own scope twice to itself
private IScope composedScope(IScope owningScope) {
//FIXME!!!!!!!!!!! nicer design, this whole method could be inside scope factory
Iterator<IScope> nextEager = eagerScopes.iterator();
if (!nextEager.hasNext())
return owningScope;

IScope result = nextEager.next();
while (nextEager.hasNext()) {
//FIXME!!!!!!!!!!! chech whether joining needs to happen - e.g. whehter they see each other
IScope next = nextEager.next();
result = ScopeFactory.createJoinedScopesView(result, next);
}
result = ScopeFactory.createJoinedScopesView(result, owningScope);

return result;
return scopesJoiner.createJoinedScopes(eagerScopes, owningScope);
}

private void leavingScope(IScope originalScope) {
if (originalScope != null) {
private void leavingScope(IScope addedScope) {
if (addedScope != null) {
eagerScopes.pop();
}
}

private IScope enteringScopeOf(Expression value) {
IScope owningScope = value.getScope();
if (owningScope != null) {
eagerScopes.push(owningScope);
}
if (owningScope==null)
return null;

if (!eagerScopes.isEmpty() && eagerScopes.peek()==eagerScopes)
return null;

eagerScopes.push(owningScope);
return owningScope;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ public boolean seesLocalDataOf(IScope otherScope) {
return getParent().seesLocalDataOf(otherScope);
}

//FIXME ! this method might be wasteful, we are looking for the same tree, so there should be no need to restart search from top for each parent
// this method might be wasteful, we are looking for the same tree, so there should
// be no need to restart search from top for each parent
public boolean seesAllDataOf(IScope otherScope) {
boolean isLocalImport = seesLocalDataOf(otherScope);
IScope parent = otherScope.getParent();
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package com.github.sommeri.less4j.core.compiler.stages;

import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;

import com.github.sommeri.less4j.core.compiler.scopes.FullMixinDefinition;
import com.github.sommeri.less4j.core.compiler.scopes.IScope;
import com.github.sommeri.less4j.core.compiler.scopes.ScopeFactory;
import com.github.sommeri.less4j.core.compiler.scopes.view.ScopeView;

public class CallerCalleeScopeJoiner {

public ScopeView joinIfIndependentAndPreserveContent(IScope callerScope, IScope bodyScope) {
// locally defined mixin does not require any other action
boolean isLocalImport = bodyScope.seesLocalDataOf(callerScope);

ScopeView result = null;
if (isLocalImport) {
// we need to copy the whole tree, because this runs inside referenced mixin scope
// snapshot and imported mixin needs to remember the scope as it is now
result = ScopeFactory.createJoinedScopesView(null, bodyScope);
} else {
// since this is non-local import, we need to join reference scope and imported mixins scope
// imported mixin needs to have access to variables defined in caller
result = ScopeFactory.createJoinedScopesView(callerScope, bodyScope);
}
result.saveLocalDataForTheWholeWayUp();
return result;
}

public IScope joinIfIndependent(IScope callerScope, IScope bodyScope) {
// locally defined mixin does not require any other action
boolean isLocallyDefined = bodyScope.seesLocalDataOf(callerScope);

if (isLocallyDefined) {
return bodyScope;
}

//join scopes
IScope result = ScopeFactory.createJoinedScopesView(callerScope, bodyScope);
return result;
}

public List<FullMixinDefinition> mixinsToImport(IScope callerScope, IScope calleeScope, List<FullMixinDefinition> unmodifiedMixinsToImport) {
List<FullMixinDefinition> result = new ArrayList<FullMixinDefinition>();
for (FullMixinDefinition mixinToImport : unmodifiedMixinsToImport) {
boolean isLocalImport = mixinToImport.getScope().seesLocalDataOf(callerScope);
ScopeView newScope = null;
if (isLocalImport) {
// we need to copy the whole tree, because this runs inside referenced mixin scope
// snapshot and imported mixin needs to remember the scope as it is now
newScope = ScopeFactory.createJoinedScopesView(null, mixinToImport.getScope());
newScope.saveLocalDataForTheWholeWayUp();
} else {
// since this is non-local import, we need to join reference scope and imported mixins scope
// imported mixin needs to have access to variables defined in caller
newScope = ScopeFactory.createJoinedScopesView(calleeScope, mixinToImport.getScope());
newScope.saveLocalDataForTheWholeWayUp();
}

result.add(new FullMixinDefinition(mixinToImport.getMixin(), newScope));
}
return result;
}

public IScope createJoinedScopes(List<IScope> parents, IScope child) {
// this could and maybe should check whether they are local to each other e.g., whther they see each other
// if they are local, parent scope could be skipped
// such logic is not implemented yet
Iterator<IScope> iterator = parents.iterator();
if (!iterator.hasNext())
return child;

IScope result = iterator.next();
while (iterator.hasNext()) {
IScope next = iterator.next();
result = ScopeFactory.createJoinedScopesView(result, next);
}
result = ScopeFactory.createJoinedScopesView(result, child);

return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.github.sommeri.less4j.core.compiler.scopes.InScopeSnapshotRunner.IFunction;
import com.github.sommeri.less4j.core.compiler.scopes.InScopeSnapshotRunner.ITask;
import com.github.sommeri.less4j.core.compiler.scopes.ScopeFactory;
import com.github.sommeri.less4j.core.compiler.scopes.ScopeManipulation;
import com.github.sommeri.less4j.core.problems.ProblemsHandler;

class MixinsSolver {
Expand All @@ -34,7 +33,7 @@ class MixinsSolver {
private final AstNodesStack semiCompiledNodes;
private final Configuration configuration;
private final DefaultGuardHelper defaultGuardHelper;
private final ScopeManipulation scopeManipulation = new ScopeManipulation();
private final CallerCalleeScopeJoiner scopeManipulation = new CallerCalleeScopeJoiner();

public MixinsSolver(ReferencesSolver parentSolver, AstNodesStack semiCompiledNodes, ProblemsHandler problemsHandler, Configuration configuration) {
this.parentSolver = parentSolver;
Expand Down Expand Up @@ -111,8 +110,10 @@ public GeneralBody buildMixinReferenceReplacement(final MixinReference reference

@Override
public void run() {
// add arguments
IScope mixinArguments = buildMixinsArguments(reference, callerScope, fullMixin);
IScope mixinWorkingScope = scopeManipulation.calculateMixinsWorkingScope(callerScope, mixinArguments, mixinScope);
mixinScope.getParent().add(mixinArguments);
IScope mixinWorkingScope = scopeManipulation.joinIfIndependent(callerScope, mixinScope);

MixinsGuardsValidator guardsValidator = new MixinsGuardsValidator(mixinWorkingScope, problemsHandler, configuration);
GuardValue guardValue = guardsValidator.evaluateGuards(mixin);
Expand Down Expand Up @@ -147,8 +148,7 @@ public void run() {
}

public GeneralBody buildDetachedRulesetReplacement(DetachedRulesetReference reference, IScope callerScope, DetachedRuleset detachedRuleset, IScope detachedRulesetScope) {
//FIXME: !!!!!!!!!!!! this should run in detachedRulesetScope parent snapshot - check whether it is the case
IScope mixinWorkingScope = scopeManipulation.calculateBodyWorkingScope(callerScope, detachedRulesetScope);
IScope mixinWorkingScope = scopeManipulation.joinIfIndependent(callerScope, detachedRulesetScope);
BodyCompilationResult compiled = resolveCalledBody(callerScope, detachedRuleset, mixinWorkingScope);
GeneralBody result = new GeneralBody(reference.getUnderlyingStructure());

Expand Down Expand Up @@ -181,12 +181,11 @@ private void declarationsAreImportant(Body result) {
}
}

//FIXME !!!! refactor and clean, unify with references
class ImportedScopeFilter implements ExpressionFilter {

private final ExpressionsEvaluator expressionEvaluator;
private final IScope importTargetScope;
private final ScopeManipulation scopeManipulation = new ScopeManipulation();
private final CallerCalleeScopeJoiner scopeManipulation = new CallerCalleeScopeJoiner();

public ImportedScopeFilter(ExpressionsEvaluator expressionEvaluator, IScope importTargetScope) {
super();
Expand All @@ -196,7 +195,6 @@ public ImportedScopeFilter(ExpressionsEvaluator expressionEvaluator, IScope impo

public Expression apply(Expression input) {
Expression result = expressionEvaluator.evaluate(input);
//FIXME: !!!!!!!! probably not necessary? expression evaluator should take care of this
IScope newScope = apply(result.getScope());
result.setScope(newScope);
return result;
Expand All @@ -206,7 +204,7 @@ private IScope apply(IScope input) {
if (input == null)
return importTargetScope;

return scopeManipulation.constructImportedBodyScope(importTargetScope, input);
return scopeManipulation.joinIfIndependentAndPreserveContent(importTargetScope, input);
}

}
Expand Down

0 comments on commit fdbffec

Please sign in to comment.