Issue 1330. Return actual Type from DartNewExpression.

Issue 1330. Return actual Type from DartNewExpression.
http://code.google.com/p/dart/issues/detail?id=1330


R=zundel@google.com
BUG=
TEST=

Committed: https://code.google.com/p/dart/source/detail?r=3609

[zundel, 2012-01-25 20:21:00]

http://codereview.chromium.org/9113064/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/ast/DartPropertyAccess.java (right):

http://codereview.chromium.org/9113064/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/ast/DartPropertyAccess.java:21: private
Type type = Types.newDynamicType();
I don't think the default initialization should be necessary.  A node that
hasn't been resolved yet should have a null type. After resolving the type
should be set.

http://codereview.chromium.org/9113064/diff/1/compiler/java/com/google/dart/c...
File compiler/java/com/google/dart/compiler/resolver/Resolver.java (right):

http://codereview.chromium.org/9113064/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/resolver/Resolver.java:1239:
recordType(node, node.getQualifier().getType());
This is correct if this from a named constructor declared like this:

class Foo {
  Foo.from() { ... };
}

main () {
  new Foo.from();
}

BUT, there is a case where the qualifier is a library name that this would blow
up on.

#library("foo")
class Foo {
  Foo(){};
}
...
#library("main")
#import("foo.dart", "foo")

main() {
  new foo.Foo();
]

and of course you might have

main() {
  new foo.Foo.from();
}

There may be a test for this already in one of the shared "Prefix" tests, if
not, we should add one.

[scheglov, 2012-01-25 21:04:14]

https://chromiumcodereview.appspot.com/9113064/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/ast/DartPropertyAccess.java (right):

https://chromiumcodereview.appspot.com/9113064/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/ast/DartPropertyAccess.java:21: private
Type type = Types.newDynamicType();
On 2012/01/25 20:21:00, zundel wrote:
> I don't think the default initialization should be necessary.  A node that
> hasn't been resolved yet should have a null type. After resolving the type
> should be set.

Done.

https://chromiumcodereview.appspot.com/9113064/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/resolver/Resolver.java (right):

https://chromiumcodereview.appspot.com/9113064/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/Resolver.java:1239:
recordType(node, node.getQualifier().getType());
There is already Prefix12Test.dart for using named constructors for classes from
libraries imported with prefix. And Prefix11Test.dart for case if not named
constructors.

Actually DartParser.parseConstructorInvocation(boolean) checks for prefix and
creates correct AST: PropAccess(TypeNode,Ident) or TypeNode(PropAccess). So,
here we know that qualifier is actually DartType.

[zundel, 2012-01-25 21:07:48]

https://chromiumcodereview.appspot.com/9113064/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/resolver/Resolver.java (right):

https://chromiumcodereview.appspot.com/9113064/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/resolver/Resolver.java:1239:
recordType(node, node.getQualifier().getType());
On 2012/01/25 21:04:14, scheglov wrote:
> There is already Prefix12Test.dart for using named constructors for classes
from
> libraries imported with prefix. And Prefix11Test.dart for case if not named
> constructors.
> 
> Actually DartParser.parseConstructorInvocation(boolean) checks for prefix and
> creates correct AST: PropAccess(TypeNode,Ident) or TypeNode(PropAccess). So,
> here we know that qualifier is actually DartType.

Huh, I would have expected the qualifier in the prefix example to resolve to the
LibraryElement.

[scheglov, 2012-01-25 21:30:41]

> > Actually DartParser.parseConstructorInvocation(boolean) checks for prefix
and
> > creates correct AST: PropAccess(TypeNode,Ident) or TypeNode(PropAccess). So,
> > here we know that qualifier is actually DartType.
> 
> Huh, I would have expected the qualifier in the prefix example to resolve to
the
> LibraryElement.

  Yes.
  But we we visit only visitTypeNode() method in case of "new
prefix.TypeName()".
  We don't visit visitPropertyAccess() in this case at all.

  As I can see x.getConstructor().accept(DartPlainVisitor) works here as
type-based switch statement. It does not perform deep visiting. We could just
write to "if" statements to achieve same result.

[zundel, 2012-01-25 21:32:42]

lgtm