From 6d388248b6cfe9daeee8896a8166d4a78b15acfb Mon Sep 17 00:00:00 2001 From: Natalie Weizenbaum Date: Thu, 13 Feb 2020 15:23:53 -0800 Subject: [PATCH] Eagerly initialize Environment._globalModules (#952) We had been lazily initializing this to be more efficient when no global modules were used, but this meant that the object wasn't shared with closures created for mixins and functions that were created when it was still `null`, so later imported forwards weren't visible to those members. --- CHANGELOG.md | 3 +++ lib/src/async_environment.dart | 26 +++++++------------------- lib/src/environment.dart | 28 ++++++++-------------------- 3 files changed, 18 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33da4ef50..4dccfd3bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ * Don't throw errors if the exact same member is loaded or forwarded from multiple modules at the same time. +* Fix a bug where imported forwarded members weren't visible in mixins and + functions that were defined before the `@import`. + ## 1.25.2 * Fix a bug where, under extremely rare circumstances, a valid variable could diff --git a/lib/src/async_environment.dart b/lib/src/async_environment.dart index 246e24264..e38e04272 100644 --- a/lib/src/async_environment.dart +++ b/lib/src/async_environment.dart @@ -40,15 +40,11 @@ class AsyncEnvironment { final Map _namespaceNodes; /// The namespaceless modules used in the current scope. - /// - /// This is `null` if there are no namespaceless modules. - Set _globalModules; + final Set _globalModules; /// A map from modules in [_globalModules] to the nodes whose spans /// indicate where those modules were originally loaded. - /// - /// This is `null` if there are no namespaceless modules. - Map _globalModuleNodes; + final Map _globalModuleNodes; /// The modules forwarded by this module. /// @@ -153,8 +149,8 @@ class AsyncEnvironment { AsyncEnvironment({bool sourceMap = false}) : _modules = {}, _namespaceNodes = {}, - _globalModules = null, - _globalModuleNodes = null, + _globalModules = {}, + _globalModuleNodes = {}, _forwardedModules = null, _forwardedModuleNodes = null, _nestedForwardedModules = null, @@ -216,8 +212,8 @@ class AsyncEnvironment { AsyncEnvironment forImport() => AsyncEnvironment._( {}, {}, - null, - null, + {}, + {}, null, null, null, @@ -240,8 +236,6 @@ class AsyncEnvironment { /// with the same name as a variable defined in this environment. void addModule(Module module, AstNode nodeWithSpan, {String namespace}) { if (namespace == null) { - _globalModules ??= {}; - _globalModuleNodes ??= {}; _globalModules.add(module); _globalModuleNodes[module] = nodeWithSpan; _allModules.add(module); @@ -337,8 +331,6 @@ class AsyncEnvironment { var forwarded = module._environment._forwardedModules; if (forwarded == null) return; - _globalModules ??= {}; - _globalModuleNodes ??= {}; _forwardedModules ??= []; _forwardedModuleNodes ??= {}; @@ -487,8 +479,6 @@ class AsyncEnvironment { /// required, since some nodes need to do real work to manufacture a source /// span. AstNode _getVariableNodeFromGlobalModule(String name) { - if (_globalModules == null) return null; - // We don't need to worry about multiple modules defining the same variable, // because that's already been checked by [getVariable]. for (var module in _globalModules) { @@ -558,7 +548,7 @@ class AsyncEnvironment { // If this module doesn't already contain a variable named [name], try // setting it in a global module. - if (!_variables.first.containsKey(name) && _globalModules != null) { + if (!_variables.first.containsKey(name)) { var moduleWithName = _fromOneModule(name, "variable", (module) => module.variables.containsKey(name) ? module : null); if (moduleWithName != null) { @@ -861,8 +851,6 @@ class AsyncEnvironment { } } - if (_globalModules == null) return null; - T value; Object identity; for (var module in _globalModules) { diff --git a/lib/src/environment.dart b/lib/src/environment.dart index 6b4ec1d9c..5e2524894 100644 --- a/lib/src/environment.dart +++ b/lib/src/environment.dart @@ -5,7 +5,7 @@ // DO NOT EDIT. This file was generated from async_environment.dart. // See tool/grind/synchronize.dart for details. // -// Checksum: db31838dbc5c44989803274acb581263e98b488d +// Checksum: df5ee8bde1eec6e47c1d025041921aba01637696 // // ignore_for_file: unused_import @@ -46,15 +46,11 @@ class Environment { final Map _namespaceNodes; /// The namespaceless modules used in the current scope. - /// - /// This is `null` if there are no namespaceless modules. - Set> _globalModules; + final Set> _globalModules; /// A map from modules in [_globalModules] to the nodes whose spans /// indicate where those modules were originally loaded. - /// - /// This is `null` if there are no namespaceless modules. - Map, AstNode> _globalModuleNodes; + final Map, AstNode> _globalModuleNodes; /// The modules forwarded by this module. /// @@ -159,8 +155,8 @@ class Environment { Environment({bool sourceMap = false}) : _modules = {}, _namespaceNodes = {}, - _globalModules = null, - _globalModuleNodes = null, + _globalModules = {}, + _globalModuleNodes = {}, _forwardedModules = null, _forwardedModuleNodes = null, _nestedForwardedModules = null, @@ -222,8 +218,8 @@ class Environment { Environment forImport() => Environment._( {}, {}, - null, - null, + {}, + {}, null, null, null, @@ -247,8 +243,6 @@ class Environment { void addModule(Module module, AstNode nodeWithSpan, {String namespace}) { if (namespace == null) { - _globalModules ??= {}; - _globalModuleNodes ??= {}; _globalModules.add(module); _globalModuleNodes[module] = nodeWithSpan; _allModules.add(module); @@ -344,8 +338,6 @@ class Environment { var forwarded = module._environment._forwardedModules; if (forwarded == null) return; - _globalModules ??= {}; - _globalModuleNodes ??= {}; _forwardedModules ??= []; _forwardedModuleNodes ??= {}; @@ -494,8 +486,6 @@ class Environment { /// required, since some nodes need to do real work to manufacture a source /// span. AstNode _getVariableNodeFromGlobalModule(String name) { - if (_globalModules == null) return null; - // We don't need to worry about multiple modules defining the same variable, // because that's already been checked by [getVariable]. for (var module in _globalModules) { @@ -565,7 +555,7 @@ class Environment { // If this module doesn't already contain a variable named [name], try // setting it in a global module. - if (!_variables.first.containsKey(name) && _globalModules != null) { + if (!_variables.first.containsKey(name)) { var moduleWithName = _fromOneModule(name, "variable", (module) => module.variables.containsKey(name) ? module : null); if (moduleWithName != null) { @@ -867,8 +857,6 @@ class Environment { } } - if (_globalModules == null) return null; - T value; Object identity; for (var module in _globalModules) {