From f95d52d8279c6b6e29579b61fddd1f5ab15c5dba Mon Sep 17 00:00:00 2001 From: "fabiomfv@google.com" Date: Tue, 8 Nov 2011 15:56:45 +0000 Subject: [PATCH] https://code.google.com/p/dart/issues/detail?id=255 Review URL: http://codereview.chromium.org//8479041 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@1317 260f80e4-7a28-3924-810f-c04153c831b5 --- .../backend/js/GenerateJavascriptAST.java | 16 +-- .../dart/compiler/parser/DartParser.java | 114 ++++++++---------- .../dart/compiler/resolver/Resolver.java | 6 +- .../compiler/resolver/ResolverErrorCode.java | 1 + .../resolver/NegativeResolverTest.java | 47 ++++++-- .../SuperMultipleInvocationsTest.dart | 15 +++ tests/co19/co19-compiler.status | 1 - 7 files changed, 115 insertions(+), 85 deletions(-) create mode 100644 compiler/javatests/com/google/dart/compiler/resolver/SuperMultipleInvocationsTest.dart diff --git a/compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java b/compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java index fcb6a747614..b9971760c05 100644 --- a/compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java +++ b/compiler/java/com/google/dart/compiler/backend/js/GenerateJavascriptAST.java @@ -869,21 +869,16 @@ public class GenerateJavascriptAST { functionStack.push(constructor.getFunction()); jsNewDeclarationsStack.push(new HashSet()); - JsInvocation constructorInvocation = maybeGenerateSuperOrRedirectCall(constructor); - boolean hasConstructorInvocation = constructorInvocation != null; - Iterator iterator = initializers.iterator(); - Iterator fieldIterator = fieldInitializers.iterator(); - - List jsInitializers = initFunction.getBody().getStatements(); - // Do the field inline initializers first. If there are any assignments in the initializer // list, they will be the last assignments. + List jsInitializers = initFunction.getBody().getStatements(); + Iterator fieldIterator = fieldInitializers.iterator(); while (fieldIterator.hasNext()) { - JsExpression initializer = generateInlineFieldInitializer(fieldIterator.next()); - jsInitializers.add(initializer.makeStmt()); + jsInitializers.add(generateInlineFieldInitializer(fieldIterator.next()).makeStmt()); } DartInvocation initInvocation = null; + Iterator iterator = initializers.iterator(); while (iterator.hasNext()) { DartInitializer initializer = iterator.next(); if (!initializer.isInvocation()) { @@ -893,7 +888,8 @@ public class GenerateJavascriptAST { } } - if (hasConstructorInvocation) { + JsInvocation constructorInvocation = maybeGenerateSuperOrRedirectCall(constructor); + if (constructorInvocation != null) { // Call the super initializer function in the initializer. // Compute the super constructor initializer to call. ConstructorElement superElement = (ConstructorElement) initInvocation.getSymbol(); diff --git a/compiler/java/com/google/dart/compiler/parser/DartParser.java b/compiler/java/com/google/dart/compiler/parser/DartParser.java index 4b45d4a19c2..842962ae23c 100644 --- a/compiler/java/com/google/dart/compiler/parser/DartParser.java +++ b/compiler/java/com/google/dart/compiler/parser/DartParser.java @@ -1023,44 +1023,70 @@ public class DartParser extends CompletionHooksParserBase { /** * 
-   * fieldInitializer
-   *     : (THIS '.')? identifier '=' conditionalExpression
-   *     | THIS ('.' identifier)? arguments
-   *     ;
-   *
+ * initializers + * : ':' superCallOrFirstFieldInitializer (',' fieldInitializer)* + * | THIS ('.' identifier) formalParameterList + * ; * + * fieldInitializer + * : (THIS '.')? identifier '=' conditionalExpression + * ; + * + * superCallOrFirstFieldInitializer + * : SUPER arguments | SUPER '.' identifier arguments + * | fieldInitializer + * ; + * + * fieldInitializer + * : (THIS '.')? identifier '=' conditionalExpression + * | THIS ('.' identifier)? arguments + * ; + * * @return true if initializer is a redirected constructor, false otherwise. */ - private boolean parseFieldInitializersOrRedirectedConstructor(List inits) { + private boolean parseInitializers(List initializers) { + expect(Token.COLON); do { - beginFieldInitializerOrRedirectedConstructor(); - boolean hasThisPrefix = optional(Token.THIS); - if (hasThisPrefix) { - if (match(Token.LPAREN)) { - return parseRedirectedConstructorInvocation(null, inits); + beginInitializer(); + if (match(Token.SUPER)) { + beginSuperInitializer(); + expect(Token.SUPER); + DartIdentifier constructor = null; + if (optional(Token.PERIOD)) { + constructor = parseIdentifier(); } - expect(Token.PERIOD); - } - DartIdentifier name = parseIdentifier(); - if (hasThisPrefix && match(Token.LPAREN)) { - return parseRedirectedConstructorInvocation(name, inits); + DartSuperConstructorInvocation superInvocation = + new DartSuperConstructorInvocation(constructor, parseArguments()); + initializers.add(done(new DartInitializer(null, done(superInvocation)))); } else { - expect(Token.ASSIGN); - boolean save = setAllowFunctionExpression(false); - DartExpression initExpr = parseExpression(); - setAllowFunctionExpression(save); - inits.add(done(new DartInitializer(name, initExpr))); + boolean hasThisPrefix = optional(Token.THIS); + if (hasThisPrefix) { + if (match(Token.LPAREN)) { + return parseRedirectedConstructorInvocation(null, initializers); + } + expect(Token.PERIOD); + } + DartIdentifier name = parseIdentifier(); + if (hasThisPrefix && match(Token.LPAREN)) { + return parseRedirectedConstructorInvocation(name, initializers); + } else { + expect(Token.ASSIGN); + boolean save = setAllowFunctionExpression(false); + DartExpression initExpr = parseExpression(); + setAllowFunctionExpression(save); + initializers.add(done(new DartInitializer(name, initExpr))); + } } } while (optional(Token.COMMA)); return false; } private boolean parseRedirectedConstructorInvocation(DartIdentifier name, - List inits) { - if (inits.isEmpty()) { - DartInvocation call = - doneWithoutConsuming(new DartRedirectConstructorInvocation(name, parseArguments())); - inits.add(done(new DartInitializer(null, call))); + List initializers) { + if (initializers.isEmpty()) { + DartRedirectConstructorInvocation redirConstructor = + new DartRedirectConstructorInvocation(name, parseArguments()); + initializers.add(done(new DartInitializer(null, doneWithoutConsuming(redirConstructor)))); return true; } else { reportUnexpectedToken(position(), Token.ASSIGN, Token.LPAREN); @@ -1068,42 +1094,6 @@ public class DartParser extends CompletionHooksParserBase { return false; } - /** - * 
-   * initializers : ':' superCallOrFirstFieldInitializer (',' fieldInitializer)*
-   *              | THIS ('.' identifier) formalParameterList ;
-   *
-   * fieldInitializer : (THIS '.')? identifier '=' conditionalExpression ;
-   *
-   * superCallOrFirstFieldInitializer : SUPER arguments | SUPER '.' identifier
-   * arguments | fieldInitializer ;
-   * 
-   *
-   * @return true if initializer is a redirect constructor, false otherwise.
-   */
-  private boolean parseInitializers(List initializers) {
-    expect(Token.COLON);
-    boolean callSuper = false;
-    if (match(Token.SUPER)) {
-      beginInitializer();
-      beginSuperInitializer();
-      expect(Token.SUPER);
-      callSuper = true;
-      DartIdentifier constructor = null;
-      if (optional(Token.PERIOD)) {
-        // Calling a super named constructor.
-        constructor = parseIdentifier();
-      }
-      DartSuperConstructorInvocation call =
-          done(new DartSuperConstructorInvocation(constructor, parseArguments()));
-      initializers.add(done(new DartInitializer(null, call)));
-    }
-    if (!callSuper || optional(Token.COMMA)) {
-      return parseFieldInitializersOrRedirectedConstructor(initializers);
-    }
-    return false;
-  }
-
   /**
    * 
    * variableDeclaration
diff --git a/compiler/java/com/google/dart/compiler/resolver/Resolver.java b/compiler/java/com/google/dart/compiler/resolver/Resolver.java
index ddc52d90a08..91e2e23c27b 100644
--- a/compiler/java/com/google/dart/compiler/resolver/Resolver.java
+++ b/compiler/java/com/google/dart/compiler/resolver/Resolver.java
@@ -1333,13 +1333,15 @@ public class Resolver {
     }
 
     private void resolveInitializers(DartMethodDefinition node) {
-      assert null != node;
       Iterator initializers = node.getInitializers().iterator();
       ConstructorElement constructorElement = null;
       while (initializers.hasNext()) {
         DartInitializer initializer = initializers.next();
         Element element = resolve(initializer);
-        if (ElementKind.of(element) == ElementKind.CONSTRUCTOR) {
+        if ((ElementKind.of(element) == ElementKind.CONSTRUCTOR) && initializer.isInvocation()) {
+          if (constructorElement != null) {
+            onError(initializer, ResolverErrorCode.SUPER_INVOCATION_NOT_UNIQUE);
+          }
           constructorElement = (ConstructorElement) element;
         }
       }
diff --git a/compiler/java/com/google/dart/compiler/resolver/ResolverErrorCode.java b/compiler/java/com/google/dart/compiler/resolver/ResolverErrorCode.java
index 1a6b82cca5e..ea6820821f0 100644
--- a/compiler/java/com/google/dart/compiler/resolver/ResolverErrorCode.java
+++ b/compiler/java/com/google/dart/compiler/resolver/ResolverErrorCode.java
@@ -98,6 +98,7 @@ public enum ResolverErrorCode implements ErrorCode {
   STATIC_METHOD_ACCESS_SUPER("Cannot use 'super' in a static method"),
   STATIC_METHOD_ACCESS_THIS("Cannot use 'this' in a static method"),
   SUPER_OUTSIDE_OF_METHOD("Cannot use 'super' outside of a method"),
+  SUPER_INVOCATION_NOT_UNIQUE("'super' must be called only once in the initialization list"),
   TOP_LEVEL_METHOD_ACCESS_SUPER("Cannot use 'super' in a top-level method"),
   TOP_LEVEL_METHOD_ACCESS_THIS("Cannot use 'this' in a top-level method"),
   TYPE_NOT_ASSIGNMENT_COMPATIBLE("%s is not assignable to %s"),
diff --git a/compiler/javatests/com/google/dart/compiler/resolver/NegativeResolverTest.java b/compiler/javatests/com/google/dart/compiler/resolver/NegativeResolverTest.java
index 1e4ff5af89b..56a72a63c34 100644
--- a/compiler/javatests/com/google/dart/compiler/resolver/NegativeResolverTest.java
+++ b/compiler/javatests/com/google/dart/compiler/resolver/NegativeResolverTest.java
@@ -10,6 +10,7 @@ import com.google.dart.compiler.ErrorCode;
 import com.google.dart.compiler.ast.DartUnit;
 import com.google.dart.compiler.testing.TestCompilerContext;
 
+import java.net.URL;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -22,29 +23,50 @@ public class NegativeResolverTest extends CompilerTestCase {
     final ErrorCode errorCode;
     final int line;
     final int column;
+
     public ErrorExpectation(ErrorCode errorCode, int line, int column) {
       this.errorCode = errorCode;
       this.line = line;
       this.column = column;
     }
-    
   }
-  
+
   private static ErrorExpectation errEx(ErrorCode errorCode, int line, int column) {
     return new ErrorExpectation(errorCode, line, column);
-    
   }
-  
+
   public void checkNumErrors(String fileName, int expectedErrorCount) {
     DartUnit unit = parseUnit(fileName);
     resolve(unit);
     assertEquals(new ArrayList(), typeErrors);
     if (errors.size() != expectedErrorCount) {
-      fail(String.format("Expected %s errors, but got %s: %s",
-                         expectedErrorCount, errors.size(), errors));
+      fail(String.format("Expected %s errors, but got %s: %s", expectedErrorCount, errors.size(),
+                         errors));
     }
   }
-  
+
+  public void checkNumErrors(String fileName,  ErrorExpectation ...expectedErrors) {
+    DartUnit unit = parseUnit(fileName);
+    resolve(unit);
+    assertEquals(expectedErrors.length, errors.size());
+    for (int i = 0; i < expectedErrors.length; i++) {
+      ErrorExpectation expectedError = expectedErrors[i];
+      DartCompilationError actualError = errors.get(i);
+      if (actualError.getErrorCode() != expectedError.errorCode
+          || actualError.getLineNumber() != expectedError.line
+          || actualError.getColumnNumber() != expectedError.column) {
+        fail(String.format(
+            "Expected %s:%s:%s, but got %s:%s:%s",
+            expectedError.errorCode,
+            expectedError.line,
+            expectedError.column,
+            actualError.getErrorCode(),
+            actualError.getLineNumber(),
+            actualError.getColumnNumber()));
+      }
+    }
+  }
+
   private void resolve(DartUnit unit) {
     unit.addTopLevelNode(ResolverTestCase.makeClass("int", null));
     unit.addTopLevelNode(ResolverTestCase.makeClass("Object", null));
@@ -54,7 +76,7 @@ public class NegativeResolverTest extends CompilerTestCase {
     unit.addTopLevelNode(ResolverTestCase.makeClass("Map", null, "K", "V"));
     ResolverTestCase.resolve(unit, getContext());
   }
-  
+
   /**
    * Parses given Dart source, runs {@link Resolver} and checks that expected errors were generated.
    */
@@ -86,7 +108,7 @@ public class NegativeResolverTest extends CompilerTestCase {
             actualError.getColumnNumber()));
       }
     }
-  }  
+  }
 
   public void testInitializer1() {
     checkNumErrors("Initializer1NegativeTest.dart", 1);
@@ -253,7 +275,12 @@ public class NegativeResolverTest extends CompilerTestCase {
   public void testRawTypesNegativeTest() {
     checkNumErrors("RawTypesNegativeTest.dart", 4);
   }
-  
+
+  public void testSuperMultipleInvocationsTest() {
+    checkNumErrors("SuperMultipleInvocationsTest.dart",
+                   errEx(ResolverErrorCode.SUPER_INVOCATION_NOT_UNIQUE, 14, 52));
+  }
+
   private TestCompilerContext getContext() {
     return new TestCompilerContext() {
       @Override
diff --git a/compiler/javatests/com/google/dart/compiler/resolver/SuperMultipleInvocationsTest.dart b/compiler/javatests/com/google/dart/compiler/resolver/SuperMultipleInvocationsTest.dart
new file mode 100644
index 00000000000..86dd34b4ac9
--- /dev/null
+++ b/compiler/javatests/com/google/dart/compiler/resolver/SuperMultipleInvocationsTest.dart
@@ -0,0 +1,15 @@
+// Copyright (c) 2011, 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.
+
+class A {
+    int a;
+    A(this.a);
+    A.foo(int x, int y);
+}
+
+class B extends A {
+    int b1;
+    int b2;
+    B(int x) : this.b1 = x, super(x), this.b2 = x, super.foo(x, x);
+}
diff --git a/tests/co19/co19-compiler.status b/tests/co19/co19-compiler.status
index 1e57b1a108b..128ac17355e 100644
--- a/tests/co19/co19-compiler.status
+++ b/tests/co19/co19-compiler.status
@@ -47,7 +47,6 @@ LangGuideTest/02_Language_Constructs/02_11_Exceptions/A04/t01: Fail
 LangGuideTest/02_Language_Constructs/02_1_Class/02_1_Class_Const_Expressions/A02/t01: Fail # Bug 5371670.
 LangGuideTest/02_Language_Constructs/02_1_Class/02_1_Class_Const_Expressions/A04/t01: Fail # Bug 5371670.
 LangGuideTest/02_Language_Constructs/02_1_Class/02_1_Class_Construction/A03/t01: Fail # Bug 5371670.
-LangGuideTest/02_Language_Constructs/02_1_Class/02_1_Class_Construction/A05/t01: Fail
 LangGuideTest/02_Language_Constructs/02_1_Class/02_1_Class_Construction/A06/t04: Fail # Bug 5371670.
 LangGuideTest/02_Language_Constructs/02_1_Class/02_1_Class_Construction/A10/t01: Fail # Bug 5371670.
 LangGuideTest/02_Language_Constructs/02_1_Class/02_1_Class_Construction/A13/t01: Fail # Bug 5371670.