Unify parsing of constructor references.

Unify parsing of constructor references.

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

[Lasse Reichstein Nielsen, 2012-10-20 11:46:05]

LGTM

[Johnni Winther, 2012-10-22 09:30:31]

https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
File dart/lib/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
dart/lib/compiler/implementation/resolution/members.dart:2886: if (type == null)
{
Document where [type] has been set.

[Johnni Winther, 2012-10-22 09:41:38]

https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
File dart/lib/compiler/implementation/closure.dart (right):

https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
dart/lib/compiler/implementation/closure.dart:369: DartType type =
elements.getType(node);
Nice!

https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
File dart/lib/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
dart/lib/compiler/implementation/resolution/members.dart:2820: DartType type;
Document the semantics and usage of [isConstContext] and [type].

https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
dart/lib/compiler/implementation/resolution/members.dart:2887: type =
e.getEnclosingClass().computeType(compiler);
I don't think this is the right type. For 'new Object()' the type is right, but
for 'new List()' the type should not be 'List<E>' but and [InterfaceType] with
no type arguments based on the 'List' element, i.e. the type 'List'.

[Johnni Winther, 2012-10-22 09:48:15]

On 2012/10/22 09:41:38, Johnni Winther wrote:
>
https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
> File dart/lib/compiler/implementation/closure.dart (right):
> 
> 
https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
> dart/lib/compiler/implementation/closure.dart:369: DartType type =
> elements.getType(node);
> Nice!
> 
>
https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
> File dart/lib/compiler/implementation/resolution/members.dart (right):
> 
>
https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
> dart/lib/compiler/implementation/resolution/members.dart:2820: DartType type;
> Document the semantics and usage of [isConstContext] and [type].
> 
>
https://codereview.chromium.org/11227007/diff/1/dart/lib/compiler/implementat...
> dart/lib/compiler/implementation/resolution/members.dart:2887: type =
> e.getEnclosingClass().computeType(compiler);
> I don't think this is the right type. For 'new Object()' the type is right,
but
> for 'new List()' the type should not be 'List<E>' but and [InterfaceType] with
> no type arguments based on the 'List' element, i.e. the type 'List'.

LGTM if the type is computed as in https://codereview.chromium.org/11230007/

[Roman, 2012-10-22 11:46:08]

With this change constructor invocation AST sometimes has TypeAnnotation, and
sometimes does not have it. When there are no type arguments involved, there is
no TypeAnnotation:
"new flib.A();"
          <NewExpression getBeginToken="new" getEndToken=")">
            <Send isPrefix="false" isPostfix="false" isIndex="false"
getBeginToken="[null]" getEndToken=")">
              <selector>
                <Send isPrefix="false" isPostfix="false" isIndex="false"
getBeginToken="[null]" getEndToken="[null]">
                  <receiver>
                    <Identifier token="flib" getBeginToken="[null]"
getEndToken="[null]"/>
                  </receiver>
                  <selector>
                    <Identifier token="A" getBeginToken="[null]"
getEndToken="[null]"/>
                  </selector>
                </Send>
              </selector>
              <argumentsNode>
                <NodeList delimiter="," getBeginToken="(" getEndToken=")"/>
              </argumentsNode>
            </Send>
          </NewExpression>

But I see TypeAnnotation if there are type arguments:
"new B<Object>();"
          <NewExpression getBeginToken="new" getEndToken=")">
            <Send isPrefix="false" isPostfix="false" isIndex="false"
getBeginToken="[null]" getEndToken=")">
              <selector>
                <TypeAnnotation getBeginToken="[null]" getEndToken="[null]">
                  <typeName>
                    <Identifier token="B" getBeginToken="[null]"
getEndToken="[null]"/>
                  </typeName>
                  <typeArguments>
                    <NodeList delimiter="," getBeginToken="&lt;"
getEndToken="&gt;">
                      <TypeAnnotation getBeginToken="[null]"
getEndToken="[null]">
                        <typeName>
                          <Identifier token="Object" getBeginToken="[null]"
getEndToken="[null]"/>
                        </typeName>
                      </TypeAnnotation>
                    </NodeList>
                  </typeArguments>
                </TypeAnnotation>
              </selector>
              <argumentsNode>
                <NodeList delimiter="," getBeginToken="(" getEndToken=")"/>
              </argumentsNode>
            </Send>
          </NewExpression>

Is this expected?

[Lasse Reichstein Nielsen, 2012-10-23 07:27:25]

I believe it's expected and intended.
In the case where the source is on the form 'foo.bar', it's impossible to tell
whether foo is a class name and bar is a constructor name, or if foo is a
library prefix and bar is a class name. In that case, making either 'foo' or
'foo.bar' a TypeAnnotation might be wrong, and determining which is the type
must be delayed until resolution.

In all other cases (including 'foo' and 'foo.bar.baz') it's possible to
determine at parsing time which part must be the type ('foo' and 'foo.bar'
respectively) and make them TypeAnnotations. I.e., it's not always without a
TypeAnnatation if there are no type arguments.

[ahe, 2012-10-23 09:25:45]

On 2012/10/23 07:27:25, Lasse Reichstein Nielsen wrote:
> I believe it's expected and intended.
> In the case where the source is on the form 'foo.bar', it's impossible to tell
> whether foo is a class name and bar is a constructor name, or if foo is a
> library prefix and bar is a class name. In that case, making either 'foo' or
> 'foo.bar' a TypeAnnotation might be wrong, and determining which is the type
> must be delayed until resolution.
> 
> In all other cases (including 'foo' and 'foo.bar.baz') it's possible to
> determine at parsing time which part must be the type ('foo' and 'foo.bar'
> respectively) and make them TypeAnnotations. I.e., it's not always without a
> TypeAnnatation if there are no type arguments.

Lasse is right. This was something we came up with after Lasse first tried
turning constructor references into a list of nodes.

[ahe, 2012-10-29 14:17:37]

PTAL

[Johnni Winther, 2012-10-29 14:20:23]

lgtm