Implementation of modified scoping for initializing formals.

In order to make an implementation of initializing formal access in
`dart2js` available as specified in issue #26655 now, this CL changes
the scope management such that initializing formals are not in scope in
the constructor body, only in the initializer list.

This is done by introducing a new notion of scopes implemented
by `ExtensionScope`: Such a scope will extend an existing
`NestedScope` rather than adding a new nested scope to it.

R=johnniwinther@google.com

Review URL: https://codereview.chromium.org/2059883002 .

pkg/compiler/lib/src/diagnostics/messages.dart

@@ -684,6 +684,26 @@ class C {
   C(this.x, int x);
 }
 
+main() {
+  new C(4, 2);
+}
+""",
+            """
+class C {
+  int x;
+  C(int x, this.x);
+}
+
+main() {
+  new C(4, 2);
+}
+""",
+            """
+class C {
+  int x;
+  C(this.x, this.x);
+}
+
 main() {
   new C(4, 2);
 }

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

@@ -32,6 +32,7 @@ import 'members.dart' show lookupInScope, ResolverVisitor;
 import 'registry.dart' show ResolutionRegistry;
 import 'resolution_common.dart' show CommonResolverVisitor;
 import 'resolution_result.dart';
+import 'scope.dart' show Scope, ExtensionScope;
 
 class InitializerResolver {
   final ResolverVisitor visitor;
@@ -294,14 +295,30 @@ class InitializerResolver {
    * Resolve all initializers of this constructor. In the case of a redirecting
    * constructor, the resolved constructor's function element is returned.
    */
-  ConstructorElement resolveInitializers() {
+  ConstructorElement resolveInitializers(
+      {bool enableInitializingFormalAccess: false}) {
     Map<dynamic /*String|int*/, ConstantExpression> defaultValues =
         <dynamic /*String|int*/, ConstantExpression>{};
     ConstructedConstantExpression constructorInvocation;
     // Keep track of all "this.param" parameters specified for constructor so
     // that we can ensure that fields are initialized only once.
     FunctionSignature functionParameters = constructor.functionSignature;
+    Scope oldScope = visitor.scope;
+    if (enableInitializingFormalAccess) {
+      // In order to get the correct detection of name clashes between all
+      // parameters (regular ones and initializing formals) we must extend
+      // the parameter scope rather than adding a new nested scope.
+      visitor.scope = new ExtensionScope(visitor.scope);
+    }
+    Link<Node> parameterNodes = (functionNode.parameters == null)
+        ? const Link<Node>()
+        : functionNode.parameters.nodes;
     functionParameters.forEachParameter((ParameterElementX element) {
+      List<Element> optionals = functionParameters.optionalParameters;
+      if (!optionals.isEmpty && element == optionals.first) {
+        NodeList nodes = parameterNodes.head;
+        parameterNodes = nodes.nodes;
+      }
       if (isConst) {
         if (element.isOptional) {
           if (element.constantCache == null) {
@@ -325,9 +342,15 @@ class InitializerResolver {
         }
       }
       if (element.isInitializingFormal) {
+        VariableDefinitions variableDefinitions = parameterNodes.head;
+        Node parameterNode = variableDefinitions.definitions.nodes.head;
         InitializingFormalElementX initializingFormal = element;
         FieldElement field = initializingFormal.fieldElement;
         checkForDuplicateInitializers(field, element.initializer);
+        if (enableInitializingFormalAccess) {
+          visitor.defineLocalVariable(parameterNode, initializingFormal);
+          visitor.addToScope(initializingFormal);
+        }
         if (isConst) {
           if (element.isNamed) {
             fieldInitializers[field] = new NamedArgumentReference(element.name);
@@ -339,6 +362,7 @@ class InitializerResolver {
           isValidAsConstant = false;
         }
       }
+      parameterNodes = parameterNodes.tail;
     });
 
     if (functionNode.initializers == null) {
@@ -430,6 +454,7 @@ class InitializerResolver {
           fieldInitializers,
           constructorInvocation);
     }
+    visitor.scope = oldScope;
     return null; // If there was no redirection always return null.
   }
 }

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

@@ -444,11 +444,6 @@ class ResolverVisitor extends MappingVisitor<ResolutionResult> {
       // fields they reference are visible, but must be resolved independently.
       if (element.isInitializingFormal) {
         registry.useElement(parameterNode, element);
-        if (compiler.options.enableInitializingFormalAccess) {
-          InitializingFormalElementX initializingFormalElementX = element;
-          defineLocalVariable(parameterNode, initializingFormalElementX);
-          addToScope(initializingFormalElementX);
-        }
       } else {
         LocalParameterElementX parameterElement = element;
         defineLocalVariable(parameterNode, parameterElement);

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';
@@ -236,15 +237,17 @@ class ResolverTask extends CompilerTask {
       ResolverVisitor visitor = visitorFor(element);
       ResolutionRegistry registry = visitor.registry;
       registry.defineFunction(tree, element);
-      visitor.setupFunction(tree, element);
+      visitor.setupFunction(tree, element); // Modifies the scope.
       processAsyncMarker(compiler, element, registry);
 
       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();
+        FunctionElement redirection = resolver.resolveInitializers(
+            enableInitializingFormalAccess:
+                compiler.options.enableInitializingFormalAccess);
         if (redirection != null) {
           resolveRedirectingConstructor(resolver, tree, element, redirection);
         }

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

@@ -9,8 +9,11 @@ import '../elements/elements.dart';
 
 abstract class Scope {
   /**
-   * Adds [element] to this scope. This operation is only allowed on mutable
-   * scopes such as [MethodScope] and [BlockScope].
+   * If an [Element] named `element.name` has already been added to this
+   * [Scope], return that element and make no changes. If no such element has
+   * been added, add the given [element] to this [Scope], and return [element].
+   * Note that this operation is only allowed on mutable scopes such as
+   * [MethodScope] and [BlockScope].
    */
   Element add(Element element);
 
@@ -123,6 +126,46 @@ abstract class MutableScope extends NestedScope {
   Element localLookup(String name) => elements[name];
 }
 
+/**
+ * [ExtensionScope] enables the creation of an extended version of an
+ * existing [NestedScope], received during construction and stored in
+ * [extendee]. An [ExtensionScope] will treat an added `element` as conflicting
+ * if an element `e` where `e.name == element.name` exists among the elements
+ * added to this [ExtensionScope], or among the ones added to [extendee]
+ * (according to `extendee.localLookup`). In this sense, it represents the
+ * union of the bindings stored locally in [elements] and the bindings in
+ * [extendee], not a new scope which is nested inside [extendee].
+ *
+ * Note that it is required that no bindings are added to [extendee] during the
+ * lifetime of this [ExtensionScope]: That would enable duplicates to be
+ * introduced into the extended scope consisting of [this] plus [extendee]
+ * without detection.
+ */
+class ExtensionScope extends Scope {
+  final NestedScope extendee;
+  final Map<String, Element> elements;
+
+  ExtensionScope(this.extendee) : this.elements = new Map<String, Element>() {
+    assert(extendee != null);
+  }
+
+  Element lookup(String name) {
+    Element result = elements[name];
+    if (result != null) return result;
+    return extendee.lookup(name);
+  }
+
+  Element add(Element newElement) {
+    if (elements.containsKey(newElement.name)) {
+      return elements[newElement.name];
+    }
+    Element existing = extendee.localLookup(newElement.name);
+    if (existing != null) return existing;
+    elements[newElement.name] = newElement;
+    return newElement;
+  }
+}
+
 class MethodScope extends MutableScope {
   final Element element;
 

tests/compiler/dart2js/message_kind_helper.dart

@@ -35,6 +35,7 @@ final Set<MessageKind> kindsWithExtraMessages = new Set<MessageKind>.from([
     MessageKind.CANNOT_MIXIN_MALFORMED,
     MessageKind.CANNOT_INSTANTIATE_ENUM,
     MessageKind.CYCLIC_TYPEDEF_ONE,
+    MessageKind.DUPLICATE_DEFINITION,
     MessageKind.EQUAL_MAP_ENTRY_KEY,
     MessageKind.FINAL_FUNCTION_TYPE_PARAMETER,
     MessageKind.FORMAL_DECLARED_CONST,

tests/language/initializing_formal_scope_test.dart

@@ -0,0 +1,25 @@
+// Copyright (c) 2016, 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.
+//
+// DartOptions=--initializing-formal-access
+
+import "package:expect/expect.dart";
+
+// Duplicate definition checks for `this.x` will check the scopes associated
+// with the constructor, not all enclosing scopes; so this is not a conflict.
+var x;
+
+class A {
+  var x;
+  A(this.x) {
+    // In the body the field is in scope, not the initializing formal;
+    // so we can use the setter.
+    x += 1;
+  }
+}
+
+main() {
+  A a = new A(2);
+  Expect.equals(a.x, 3);
+}

tests/language/language.status

@@ -56,10 +56,10 @@ generic_methods_function_type_test: CompiletimeError # Issue 25869
 generic_methods_type_expression_test: CompiletimeError # Issue 25869
 
 # Experimental feature: Use initializing formals in initializers and constructor body.
-initializing_formal_access_test: CompiletimeError # TODO(eernst): Create a suitable sdk issue.
-initializing_formal_capture_test: CompiletimeError # TODO(eernst): Create a suitable sdk issue.
-initializing_formal_final_test: CompiletimeError # TODO(eernst): Create a suitable sdk issue.
-initializing_formal_type_test: CompiletimeError # TODO(eernst): Create a suitable sdk issue.
+initializing_formal_access_test: CompiletimeError # Issue 26656
+initializing_formal_capture_test: CompiletimeError # Issue 26656
+initializing_formal_final_test: CompiletimeError # Issue 26656
+initializing_formal_type_test: CompiletimeError # Issue 26656
 
 [ ($compiler == none || $compiler == precompiler || $compiler == dart2app || $compiler == dart2appjit) && ($runtime == vm || $runtime == dart_precompiled || $runtime == dart_app) ]
 
@@ -111,10 +111,10 @@ generic_methods_function_type_test: RuntimeError # Issue 25869
 generic_methods_type_expression_test: RuntimeError # Issue 25869
 
 # Experimental feature: Use initializing formals in initializers and constructor body.
-initializing_formal_access_test: RuntimeError # TODO(eernst): create a suitable sdk issue.
-initializing_formal_capture_test: RuntimeError # TODO(eernst): create a suitable sdk issue.
-initializing_formal_final_test: RuntimeError # TODO(eernst): create a suitable sdk issue.
-initializing_formal_type_test: RuntimeError # TODO(eernst): create a suitable sdk issue.
+initializing_formal_access_test: RuntimeError # Issue 26656
+initializing_formal_capture_test: RuntimeError # Issue 26656
+initializing_formal_final_test: RuntimeError # Issue 26656
+initializing_formal_type_test: RuntimeError # Issue 26656
 
 config_import_test: Skip  # Issue 26250
 

tests/language/language_analyzer2.status

@@ -513,8 +513,8 @@ generic_methods_function_type_test: CompiletimeError # Issue 25868
 generic_methods_type_expression_test: CompiletimeError # Issue 25868
 
 # Experimental feature: Use initializing formals in initializers and constructor body.
-initializing_formal_access_test: CompiletimeError # TODO(eernst): Create a suitable sdk issue.
-initializing_formal_capture_test: CompiletimeError # TODO(eernst): Create a suitable sdk issue.
-initializing_formal_final_test: CompiletimeError # TODO(eernst): Create a suitable sdk issue.
-initializing_formal_promotion_test: StaticWarning # TODO(eernst): Create a suitable sdk issue.
-initializing_formal_type_test: CompiletimeError # TODO(eernst): Create a suitable sdk issue.
+initializing_formal_access_test: CompiletimeError # Issue 26658
+initializing_formal_capture_test: CompiletimeError # Issue 26658
+initializing_formal_final_test: CompiletimeError # Issue 26658
+initializing_formal_promotion_test: StaticWarning # Issue 26658
+initializing_formal_type_test: CompiletimeError # Issue 26658