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

Jump to bottom

Always use a non-null map for _EvaluateVisitor._configuration #827

Merged
merged 2 commits into from Sep 27, 2019
Merged

Always use a non-null map for _EvaluateVisitor._configuration #827

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1063cb1
Always use a non-null map for _EvaluateVisitor._configuration
nex3 Sep 24, 2019
048248c
Merge branch 'feature.use' into forward-and-use
nex3 Sep 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 19 additions & 7 deletions lib/src/visitor/async_evaluate.dart
View file
Open in desktop
Expand Up @@ -253,7 +253,11 @@ class _EvaluateVisitor

/// A map from variable names to the values that override their `!default`
/// definitions in this module.
Map<String, _ConfiguredValue> _configuration;
///
/// If this is empty, that indicates that the current module is not confiured.
/// Note that it may be unmodifiable when empty, in which case [Map.remove]
/// must not be called.
var _configuration = const <String, _ConfiguredValue>{};

/// Creates a new visitor.
///
Expand Down Expand Up @@ -521,9 +525,11 @@ class _EvaluateVisitor
{Uri baseUrl,
Map<String, _ConfiguredValue> configuration,
bool namesInErrors = false}) async {
configuration ??= const {};

var builtInModule = _builtInModules[url];
if (builtInModule != null) {
if (configuration != null || _configuration != null) {
if (configuration.isNotEmpty || _configuration.isNotEmpty) {
throw _exception(
namesInErrors
? "Built-in module $url can't be configured."
Expand Down Expand Up @@ -583,11 +589,12 @@ class _EvaluateVisitor
Future<Module> _execute(AsyncImporter importer, Stylesheet stylesheet,
{Map<String, _ConfiguredValue> configuration,
bool namesInErrors = false}) async {
configuration ??= const {};
var url = stylesheet.span.sourceUrl;

var alreadyLoaded = _modules[url];
if (alreadyLoaded != null) {
if (configuration != null || _configuration != null) {
if (configuration.isNotEmpty || _configuration.isNotEmpty) {
throw _exception(namesInErrors
? "${p.prettyUri(url)} was already loaded, so it can't be "
"configured using \"with\"."
Expand Down Expand Up @@ -630,7 +637,7 @@ class _EvaluateVisitor
_atRootExcludingStyleRule = false;
_inKeyframes = false;

if (configuration != null) _configuration = Map.of(configuration);
if (configuration.isNotEmpty) _configuration = Map.of(configuration);

await visitStylesheet(stylesheet);
css = _outOfOrderImports == null
Expand All @@ -651,7 +658,7 @@ class _EvaluateVisitor
_atRootExcludingStyleRule = oldAtRootExcludingStyleRule;
_inKeyframes = oldInKeyframes;

if (configuration != null && _configuration.isNotEmpty) {
if (configuration.isNotEmpty && _configuration.isNotEmpty) {
throw _exception(
namesInErrors
? "\$${_configuration.keys.first} was not declared with "
Expand Down Expand Up @@ -1201,7 +1208,7 @@ class _EvaluateVisitor
// configuration variable is used by removing it even when the underlying
// map is wrapped.
var oldConfiguration = _configuration;
if (_configuration != null) {
if (_configuration.isNotEmpty) {
if (node.prefix != null) {
_configuration = UnprefixedMapView(_configuration, node.prefix);
}
Expand Down Expand Up @@ -1720,7 +1727,12 @@ class _EvaluateVisitor
Future<Value> visitVariableDeclaration(VariableDeclaration node) async {
if (node.isGuarded) {
if (node.namespace == null && _environment.atRoot) {
var override = _configuration?.remove(node.name);
// Explicitly check whether [_configuration] is empty because if it is,
// it may be a constant map which doesn't support `remove()`.
//
// See also dart-lang/sdk#38540.
var override =
_configuration.isEmpty ? null : _configuration.remove(node.name);
if (override != null) {
_addExceptionSpan(node, () {
_environment.setVariable(
Expand Down
28 changes: 20 additions & 8 deletions lib/src/visitor/evaluate.dart
View file
Open in desktop
Expand Up @@ -5,7 +5,7 @@
// DO NOT EDIT. This file was generated from async_evaluate.dart.
// See tool/grind/synchronize.dart for details.
//
// Checksum: 5c9f270ef574f9c6062421ed1866af3d07672b46
// Checksum: 81a3519c2d9182c91da1907a5129f3810c0ecdb5
//
// ignore_for_file: unused_import

Expand Down Expand Up @@ -261,7 +261,11 @@ class _EvaluateVisitor

/// A map from variable names to the values that override their `!default`
/// definitions in this module.
Map<String, _ConfiguredValue> _configuration;
///
/// If this is empty, that indicates that the current module is not confiured.
/// Note that it may be unmodifiable when empty, in which case [Map.remove]
/// must not be called.
var _configuration = const <String, _ConfiguredValue>{};

/// Creates a new visitor.
///
Expand Down Expand Up @@ -527,9 +531,11 @@ class _EvaluateVisitor
{Uri baseUrl,
Map<String, _ConfiguredValue> configuration,
bool namesInErrors = false}) {
configuration ??= const {};

var builtInModule = _builtInModules[url];
if (builtInModule != null) {
if (configuration != null || _configuration != null) {
if (configuration.isNotEmpty || _configuration.isNotEmpty) {
throw _exception(
namesInErrors
? "Built-in module $url can't be configured."
Expand Down Expand Up @@ -589,11 +595,12 @@ class _EvaluateVisitor
Module<Callable> _execute(Importer importer, Stylesheet stylesheet,
{Map<String, _ConfiguredValue> configuration,
bool namesInErrors = false}) {
configuration ??= const {};
var url = stylesheet.span.sourceUrl;

var alreadyLoaded = _modules[url];
if (alreadyLoaded != null) {
if (configuration != null || _configuration != null) {
if (configuration.isNotEmpty || _configuration.isNotEmpty) {
throw _exception(namesInErrors
? "${p.prettyUri(url)} was already loaded, so it can't be "
"configured using \"with\"."
Expand Down Expand Up @@ -636,7 +643,7 @@ class _EvaluateVisitor
_atRootExcludingStyleRule = false;
_inKeyframes = false;

if (configuration != null) _configuration = Map.of(configuration);
if (configuration.isNotEmpty) _configuration = Map.of(configuration);

visitStylesheet(stylesheet);
css = _outOfOrderImports == null
Expand All @@ -657,7 +664,7 @@ class _EvaluateVisitor
_atRootExcludingStyleRule = oldAtRootExcludingStyleRule;
_inKeyframes = oldInKeyframes;

if (configuration != null && _configuration.isNotEmpty) {
if (configuration.isNotEmpty && _configuration.isNotEmpty) {
throw _exception(
namesInErrors
? "\$${_configuration.keys.first} was not declared with "
Expand Down Expand Up @@ -1202,7 +1209,7 @@ class _EvaluateVisitor
// configuration variable is used by removing it even when the underlying
// map is wrapped.
var oldConfiguration = _configuration;
if (_configuration != null) {
if (_configuration.isNotEmpty) {
if (node.prefix != null) {
_configuration = UnprefixedMapView(_configuration, node.prefix);
}
Expand Down Expand Up @@ -1714,7 +1721,12 @@ class _EvaluateVisitor
Value visitVariableDeclaration(VariableDeclaration node) {
if (node.isGuarded) {
if (node.namespace == null && _environment.atRoot) {
var override = _configuration?.remove(node.name);
// Explicitly check whether [_configuration] is empty because if it is,
// it may be a constant map which doesn't support `remove()`.
//
// See also dart-lang/sdk#38540.
var override =
_configuration.isEmpty ? null : _configuration.remove(node.name);
if (override != null) {
_addExceptionSpan(node, () {
_environment.setVariable(
Expand Down