Don't create scopes when nothing is declared

This seems to provide a fairly minimal speed boost, but it's more than
nothing. It's also a pretty easy change now that we have
ParentStatement.
This commit is contained in:
Natalie Weizenbaum 2017-12-02 14:36:52 -08:00
parent 3de6680bcb
commit 3de531b9bc
6 changed files with 142 additions and 57 deletions

View File

@ -7,6 +7,9 @@ import 'package:source_span/source_span.dart';
import '../../../visitor/interface/statement.dart';
import '../expression.dart';
import '../statement.dart';
import 'function_rule.dart';
import 'mixin_rule.dart';
import 'variable_declaration.dart';
/// An `@if` rule.
///
@ -53,12 +56,20 @@ class IfClause {
/// The statements to evaluate if this clause matches.
final List<Statement> children;
IfClause(this.expression, Iterable<Statement> children)
: children = new List.unmodifiable(children);
/// Whether any of [children] is a variable, function, or mixin declaration.
final bool hasDeclarations;
IfClause(Expression expression, Iterable<Statement> children)
: this._(expression, new List.unmodifiable(children));
IfClause.last(Iterable<Statement> children)
: expression = null,
children = new List.unmodifiable(children);
: this._(null, new List.unmodifiable(children));
IfClause._(this.expression, this.children)
: hasDeclarations = children.any((child) =>
child is VariableDeclaration ||
child is FunctionRule ||
child is MixinRule);
String toString() =>
(expression == null ? "@else" : "@if $expression") +

View File

@ -3,11 +3,22 @@
// https://opensource.org/licenses/MIT.
import '../statement.dart';
import 'function_rule.dart';
import 'mixin_rule.dart';
import 'variable_declaration.dart';
/// A [Statement] that can have child statements.
abstract class ParentStatement implements Statement {
/// The child statements of this statement.
final List<Statement> children;
ParentStatement(this.children);
/// Whether any of [children] is a variable, function, or mixin declaration.
final bool hasDeclarations;
ParentStatement(this.children)
: hasDeclarations = children?.any((child) =>
child is VariableDeclaration ||
child is FunctionRule ||
child is MixinRule) ??
false;
}

View File

@ -82,11 +82,11 @@ class AsyncEnvironment {
bool get inMixin => _inMixin;
var _inMixin = false;
/// Whether the environment is currently in a semi-global scope.
/// Whether the environment is currently in a global or semi-global scope.
///
/// A semi-global scope can assign to global variables, but it doesn't declare
/// them by default.
var _inSemiGlobalScope = false;
var _inSemiGlobalScope = true;
AsyncEnvironment()
: _variables = [normalizedMap()],
@ -279,10 +279,32 @@ class AsyncEnvironment {
/// Variables, functions, and mixins declared in a given scope are
/// inaccessible outside of it. If [semiGlobal] is passed, this scope can
/// assign to global variables without a `!global` declaration.
Future<T> scope<T>(Future<T> callback(), {bool semiGlobal: false}) async {
semiGlobal = semiGlobal && (_inSemiGlobalScope || _variables.length == 1);
///
/// If [when] is false, this doesn't create a new scope and instead just
/// executes [callback] and returns its result.
Future<T> scope<T>(Future<T> callback(),
{bool semiGlobal: false, bool when: true}) async {
if (!when) {
// We still have to track semi-globalness so that
//
// div {
// @if ... {
// $x: y;
// }
// }
//
// doesn't assign to the global scope.
var wasInSemiGlobalScope = _inSemiGlobalScope;
_inSemiGlobalScope = semiGlobal;
try {
return await callback();
} finally {
_inSemiGlobalScope = wasInSemiGlobalScope;
}
}
semiGlobal = semiGlobal && _inSemiGlobalScope;
// TODO: avoid creating a new scope if no variables are declared.
var wasInSemiGlobalScope = _inSemiGlobalScope;
_inSemiGlobalScope = semiGlobal;
_variables.add(normalizedMap());

View File

@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_environment.dart.
// See tool/synchronize.dart for details.
//
// Checksum: 97410abbd78c3bbc9899f3ac460cc0736218bfe3
// Checksum: 4669f41a70664bd5f391c6b8627264a5d0ad8f6c
import 'ast/sass.dart';
import 'callable.dart';
@ -85,11 +85,11 @@ class Environment {
bool get inMixin => _inMixin;
var _inMixin = false;
/// Whether the environment is currently in a semi-global scope.
/// Whether the environment is currently in a global or semi-global scope.
///
/// A semi-global scope can assign to global variables, but it doesn't declare
/// them by default.
var _inSemiGlobalScope = false;
var _inSemiGlobalScope = true;
Environment()
: _variables = [normalizedMap()],
@ -282,10 +282,31 @@ class Environment {
/// Variables, functions, and mixins declared in a given scope are
/// inaccessible outside of it. If [semiGlobal] is passed, this scope can
/// assign to global variables without a `!global` declaration.
T scope<T>(T callback(), {bool semiGlobal: false}) {
semiGlobal = semiGlobal && (_inSemiGlobalScope || _variables.length == 1);
///
/// If [when] is false, this doesn't create a new scope and instead just
/// executes [callback] and returns its result.
T scope<T>(T callback(), {bool semiGlobal: false, bool when: true}) {
if (!when) {
// We still have to track semi-globalness so that
//
// div {
// @if ... {
// $x: y;
// }
// }
//
// doesn't assign to the global scope.
var wasInSemiGlobalScope = _inSemiGlobalScope;
_inSemiGlobalScope = semiGlobal;
try {
return callback();
} finally {
_inSemiGlobalScope = wasInSemiGlobalScope;
}
}
semiGlobal = semiGlobal && _inSemiGlobalScope;
// TODO: avoid creating a new scope if no variables are declared.
var wasInSemiGlobalScope = _inSemiGlobalScope;
_inSemiGlobalScope = semiGlobal;
_variables.add(normalizedMap());

View File

@ -327,7 +327,7 @@ class _EvaluateVisitor
for (var child in node.children) {
await child.accept(this);
}
});
}, when: node.hasDeclarations);
return null;
}
@ -341,7 +341,7 @@ class _EvaluateVisitor
}
if (outerCopy != null) root.addChild(outerCopy);
await _scopeForAtRoot(innerCopy ?? root, query, included)(() async {
await _scopeForAtRoot(node, innerCopy ?? root, query, included)(() async {
for (var child in node.children) {
await child.accept(this);
}
@ -386,14 +386,14 @@ class _EvaluateVisitor
/// This returns a callback that adjusts various instance variables for its
/// duration, based on which rules are excluded by [query]. It always assigns
/// [_parent] to [newParent].
_ScopeCallback _scopeForAtRoot(CssParentNode newParent, AtRootQuery query,
List<CssParentNode> included) {
_ScopeCallback _scopeForAtRoot(AtRootRule node, CssParentNode newParent,
AtRootQuery query, List<CssParentNode> included) {
var scope = (Future callback()) async {
// We can't use [_withParent] here because it'll add the node to the tree
// in the wrong place.
var oldParent = _parent;
_parent = newParent;
await _environment.scope(callback);
await _environment.scope(callback, when: node.hasDeclarations);
_parent = oldParent;
};
@ -488,7 +488,7 @@ class _EvaluateVisitor
for (var child in node.children) {
await child.accept(this);
}
});
}, when: node.hasDeclarations);
_declarationName = oldDeclarationName;
}
@ -588,9 +588,11 @@ class _EvaluateVisitor
for (var child in node.children) {
await child.accept(this);
}
});
}, scopeWhen: false);
}
}, through: (node) => node is CssStyleRule);
},
through: (node) => node is CssStyleRule,
scopeWhen: node.hasDeclarations);
_inUnknownAtRule = wasInUnknownAtRule;
_inKeyframes = wasInKeyframes;
@ -645,7 +647,8 @@ class _EvaluateVisitor
return await _environment.scope(
() => _handleReturn<Statement>(
clause.children, (child) => child.accept(this)),
semiGlobal: true);
semiGlobal: true,
when: clause.hasDeclarations);
}
Future<Value> visitImportRule(ImportRule node) async {
@ -878,10 +881,12 @@ class _EvaluateVisitor
for (var child in node.children) {
await child.accept(this);
}
});
}, scopeWhen: false);
}
});
}, through: (node) => node is CssStyleRule || node is CssMediaRule);
},
through: (node) => node is CssStyleRule || node is CssMediaRule,
scopeWhen: node.hasDeclarations);
return null;
}
@ -931,7 +936,9 @@ class _EvaluateVisitor
for (var child in node.children) {
await child.accept(this);
}
}, through: (node) => node is CssStyleRule);
},
through: (node) => node is CssStyleRule,
scopeWhen: node.hasDeclarations);
return null;
}
@ -955,7 +962,9 @@ class _EvaluateVisitor
await child.accept(this);
}
});
}, through: (node) => node is CssStyleRule);
},
through: (node) => node is CssStyleRule,
scopeWhen: node.hasDeclarations);
_atRootExcludingStyleRule = oldAtRootExcludingStyleRule;
if (!_inStyleRule) {
@ -992,7 +1001,9 @@ class _EvaluateVisitor
}
});
}
}, through: (node) => node is CssStyleRule);
},
through: (node) => node is CssStyleRule,
scopeWhen: node.hasDeclarations);
return null;
}
@ -1068,7 +1079,7 @@ class _EvaluateVisitor
if (result != null) return result;
}
return null;
}, semiGlobal: true);
}, semiGlobal: true, when: node.hasDeclarations);
}
// ## Expressions
@ -1250,9 +1261,6 @@ class _EvaluateVisitor
_verifyArguments(
positional.length, named, callable.declaration.arguments, span);
// TODO: if we get here and there are no rest params involved, mark
// the callable as fast-path and don't do error checking or extra
// allocations for future calls.
var declaredArguments = callable.declaration.arguments.arguments;
var minLength = math.min(positional.length, declaredArguments.length);
for (var i = 0; i < minLength; i++) {
@ -1633,9 +1641,11 @@ class _EvaluateVisitor
/// If [through] is passed, [node] is added as a child of the first parent for
/// which [through] returns `false`. That parent is copied unless it's the
/// lattermost child of its parent.
///
/// Runs [callback] in a new environment scope unless [scopeWhen] is false.
Future<T> _withParent<S extends CssParentNode, T>(
S node, Future<T> callback(),
{bool through(CssNode node)}) async {
{bool through(CssNode node), bool scopeWhen: true}) async {
var oldParent = _parent;
// Go up through parents that match [through].
@ -1657,7 +1667,7 @@ class _EvaluateVisitor
parent.addChild(node);
_parent = node;
var result = await _environment.scope(callback);
var result = await _environment.scope(callback, when: scopeWhen);
_parent = oldParent;
return result;

View File

@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_evaluate.dart.
// See tool/synchronize.dart for details.
//
// Checksum: 33e96540f3e5999ed172d168221b27910e8672b1
// Checksum: cdeeec2634c97638aef571043c0be1e99f22d7d5
import 'dart:math' as math;
@ -328,7 +328,7 @@ class _EvaluateVisitor
for (var child in node.children) {
child.accept(this);
}
});
}, when: node.hasDeclarations);
return null;
}
@ -342,7 +342,7 @@ class _EvaluateVisitor
}
if (outerCopy != null) root.addChild(outerCopy);
_scopeForAtRoot(innerCopy ?? root, query, included)(() {
_scopeForAtRoot(node, innerCopy ?? root, query, included)(() {
for (var child in node.children) {
child.accept(this);
}
@ -387,14 +387,14 @@ class _EvaluateVisitor
/// This returns a callback that adjusts various instance variables for its
/// duration, based on which rules are excluded by [query]. It always assigns
/// [_parent] to [newParent].
_ScopeCallback _scopeForAtRoot(CssParentNode newParent, AtRootQuery query,
List<CssParentNode> included) {
_ScopeCallback _scopeForAtRoot(AtRootRule node, CssParentNode newParent,
AtRootQuery query, List<CssParentNode> included) {
var scope = (void callback()) {
// We can't use [_withParent] here because it'll add the node to the tree
// in the wrong place.
var oldParent = _parent;
_parent = newParent;
_environment.scope(callback);
_environment.scope(callback, when: node.hasDeclarations);
_parent = oldParent;
};
@ -487,7 +487,7 @@ class _EvaluateVisitor
for (var child in node.children) {
child.accept(this);
}
});
}, when: node.hasDeclarations);
_declarationName = oldDeclarationName;
}
@ -583,9 +583,11 @@ class _EvaluateVisitor
for (var child in node.children) {
child.accept(this);
}
});
}, scopeWhen: false);
}
}, through: (node) => node is CssStyleRule);
},
through: (node) => node is CssStyleRule,
scopeWhen: node.hasDeclarations);
_inUnknownAtRule = wasInUnknownAtRule;
_inKeyframes = wasInKeyframes;
@ -640,7 +642,8 @@ class _EvaluateVisitor
return _environment.scope(
() => _handleReturn<Statement>(
clause.children, (child) => child.accept(this)),
semiGlobal: true);
semiGlobal: true,
when: clause.hasDeclarations);
}
Value visitImportRule(ImportRule node) {
@ -872,10 +875,12 @@ class _EvaluateVisitor
for (var child in node.children) {
child.accept(this);
}
});
}, scopeWhen: false);
}
});
}, through: (node) => node is CssStyleRule || node is CssMediaRule);
},
through: (node) => node is CssStyleRule || node is CssMediaRule,
scopeWhen: node.hasDeclarations);
return null;
}
@ -922,7 +927,9 @@ class _EvaluateVisitor
for (var child in node.children) {
child.accept(this);
}
}, through: (node) => node is CssStyleRule);
},
through: (node) => node is CssStyleRule,
scopeWhen: node.hasDeclarations);
return null;
}
@ -946,7 +953,9 @@ class _EvaluateVisitor
child.accept(this);
}
});
}, through: (node) => node is CssStyleRule);
},
through: (node) => node is CssStyleRule,
scopeWhen: node.hasDeclarations);
_atRootExcludingStyleRule = oldAtRootExcludingStyleRule;
if (!_inStyleRule) {
@ -983,7 +992,9 @@ class _EvaluateVisitor
}
});
}
}, through: (node) => node is CssStyleRule);
},
through: (node) => node is CssStyleRule,
scopeWhen: node.hasDeclarations);
return null;
}
@ -1058,7 +1069,7 @@ class _EvaluateVisitor
if (result != null) return result;
}
return null;
}, semiGlobal: true);
}, semiGlobal: true, when: node.hasDeclarations);
}
// ## Expressions
@ -1231,9 +1242,6 @@ class _EvaluateVisitor
_verifyArguments(
positional.length, named, callable.declaration.arguments, span);
// TODO: if we get here and there are no rest params involved, mark
// the callable as fast-path and don't do error checking or extra
// allocations for future calls.
var declaredArguments = callable.declaration.arguments.arguments;
var minLength = math.min(positional.length, declaredArguments.length);
for (var i = 0; i < minLength; i++) {
@ -1604,8 +1612,10 @@ class _EvaluateVisitor
/// If [through] is passed, [node] is added as a child of the first parent for
/// which [through] returns `false`. That parent is copied unless it's the
/// lattermost child of its parent.
///
/// Runs [callback] in a new environment scope unless [scopeWhen] is false.
T _withParent<S extends CssParentNode, T>(S node, T callback(),
{bool through(CssNode node)}) {
{bool through(CssNode node), bool scopeWhen: true}) {
var oldParent = _parent;
// Go up through parents that match [through].
@ -1627,7 +1637,7 @@ class _EvaluateVisitor
parent.addChild(node);
_parent = node;
var result = _environment.scope(callback);
var result = _environment.scope(callback, when: scopeWhen);
_parent = oldParent;
return result;