Implementation of modified scoping for initializing formals.

pkg/compiler/lib/src/resolution/resolution.dart

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library dart2js.resolution;
 
 import 'dart:collection' show Queue;
 
 import '../common.dart';
 import '../common/names.dart' show Identifiers;
@@ skipping 35 matching lines @@
 import '../tree/tree.dart';
 import '../universe/call_structure.dart' show CallStructure;
 import '../universe/use.dart' show StaticUse, TypeUse;
 import '../universe/world_impact.dart' show WorldImpact;
 import '../util/util.dart' show Link, Setlet;
 import 'class_hierarchy.dart';
 import 'class_members.dart' show MembersCreator;
 import 'constructors.dart';
 import 'members.dart';
 import 'registry.dart';
+import 'scope.dart' show MutableScope;
 import 'signatures.dart';
 import 'tree_elements.dart';
 import 'typedefs.dart';
 
 class ResolverTask extends CompilerTask {
   final ConstantCompiler constantCompiler;
   final Compiler compiler;
 
   ResolverTask(Compiler compiler, this.constantCompiler)
       : compiler = compiler,
@@ skipping 175 matching lines @@
 
       if (element.isGenerativeConstructor) {
         // Even if there is no initializer list we still have to do the
         // resolution in case there is an implicit super constructor call.
         InitializerResolver resolver =
             new InitializerResolver(visitor, element, tree);
         FunctionElement redirection = resolver.resolveInitializers();
         if (redirection != null) {
           resolveRedirectingConstructor(resolver, tree, element, redirection);
         }
+        if (compiler.options.enableInitializingFormalAccess) {

[Johnni Winther, 2016/06/13 08:56:48]

There is a better approach:

1. Create a subscope for resolution of the initializers in
ConstructorResolver.resolveInitializers (in resolution/constructors.dart). This
should be a BlockScope whose parent is the scope containing the normal
parameters; i.e. the current scope at that point.
2. Move the adding of initializing formals to the scope from
ResolverVisitor.setupFunction (in resolution/members.dart) to
ConstructorResolver.resolveInitializers. Remember to restore the current scope
to the scope containing the normal parameters after resolution of the
initializers; thus 'removing' the initializing formals from the scope used to
resolve the constructor body.

[eernst, 2016/06/14 15:01:51]

That sounds like a much better idea, but there are still some issues. I'm OOO
for the next 7 workdays and I can't finish resolving it right now, so here is a
listing of issues that came up, just so we have them on the table. Then we can
return to them later.

First a typo---there is no `ConstructorResolver.resolveInitializers`, but there
is an `InitializerResolver.resolveInitializers`; I'll assume that's the one you
meant.

There are no cases (that I can spot) where any specific number of scopes is
removed, but the pattern from `ResolverVisitor.visitIn` is used in several cases
where the scope must be extended locally (if, while, blocks in bodies): Remember
the `oldScope` at some point and later use `scope = oldScope` to restore it.
This will work no matter how many extra layers the intermediate code has added,
but we must obviously check that we don't return early or otherwise fail to
execute the scope restore operation). So it looks like we can do that.

However, we must still produce the 'duplicate definition' errors that would
arise if the initializing formal parameters were added to the parameter scope.
We could add something like a
`MappingVisitor<T>.checkIfAddingToScopeCreatesDuplicates` method which would do
the same thing as `MappingVisitor<T>.addToScope`, except that it wouldn't
actually add the argument to the scope.

But if we now proceed and try to perform `visitor.defineLocalVariable` and
`visitor.addToScope` in `resolveInitializers`, it is (1) not so nice that we
break the encapsulation of `visitor`, (2) seemingly impossible to get hold of
the `parameterNode` needed as an argument to `defineLocalVariable`, unless we
duplicate a rather long sequence of lookups that `setupFunction` performs in
order to get to that `parameterNode`, and that would only be possible if we pass
the same `FunctionExpression node` to `resolveInitializers` that we passed to
`setupFunction`.

So there are still several issues to juggle before this will fit nicely into the
context (and I'll get back to them in about 1.5 weeks).

PS: Yes, this is not a normal treatment of scoping, but the "obvious" approach
(which is what I implemented at first) would break so much code that Florian
preferred to change the scope rule instead. So there will be a need to go around
a tight corner here and there.

+          // Ensure that the initializing formal parameters will not be in scope
+          // when the body of the constructor is resolved.
+          // TODO(eernst): The following is a temporary solution. An acceptable
+          // solution requires some more thinking & refactoring: We don't
+          // otherwise delete from scopes, it introduces ordering dependencies,
+          // etc.etc.etc.
+          assert(visitor.scope is MutableScope);
+          MutableScope mutableScope = visitor.scope;
+          List<String> names = mutableScope.elements.keys.toList();
+          for (String name in names) {
+            if (mutableScope.elements[name] is InitializingFormalElement) {
+              mutableScope.elements.remove(name);
+            }
+          }
+        }
       } else if (tree.initializers != null) {
         reporter.reportErrorMessage(
             tree, MessageKind.FUNCTION_WITH_INITIALIZER);
       }
 
       if (!compiler.options.analyzeSignaturesOnly ||
           tree.isRedirectingFactory) {
         // We need to analyze the redirecting factory bodies to ensure that
         // we can analyze compile-time constants.
         visitor.visit(tree.body);
@@ skipping 860 matching lines @@
   TreeElements get treeElements {
     assert(invariant(this, _treeElements != null,
         message: "TreeElements have not been computed for $this."));
     return _treeElements;
   }
 
   void reuseElement() {
     _treeElements = null;
   }
 }