Implementation of modified scoping for initializing formals.

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

Index: pkg/compiler/lib/src/resolution/resolution.dart
diff --git a/pkg/compiler/lib/src/resolution/resolution.dart b/pkg/compiler/lib/src/resolution/resolution.dart
index f1ca6d02630b1314b5480dbdf9430ed0556a8a0b..81848d267bc65dfa7226af6bc5b575c1d1b3e17c 100644
--- a/pkg/compiler/lib/src/resolution/resolution.dart
+++ b/pkg/compiler/lib/src/resolution/resolution.dart
@@ -53,6 +53,7 @@ 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';
@@ -248,6 +249,22 @@ class ResolverTask extends CompilerTask {
         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);