Address the remaining review comments on http://chromiumcodereview.appspot.com/9431029.

Address the remaining review comments on http://chromiumcodereview.appspot.com/9431029.

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

[ahe, 2012-04-19 08:20:06]

http://codereview.chromium.org/10115026/diff/2001/lib/compiler/implementation...
File lib/compiler/implementation/elements/elements.dart (right):

http://codereview.chromium.org/10115026/diff/2001/lib/compiler/implementation...
lib/compiler/implementation/elements/elements.dart:346: cachedType = new
InterfaceType(name, this);
This could be set in the constructor.

http://codereview.chromium.org/10115026/diff/2001/lib/compiler/implementation...
File lib/compiler/implementation/resolver.dart (right):

http://codereview.chromium.org/10115026/diff/2001/lib/compiler/implementation...
lib/compiler/implementation/resolver.dart:1111: type =
element.computeType(compiler);
I think you have a different design in mind than the one we had in dartc. I
don't know if this is intentional or not, but if it is, I'd like to know what
your plan is.

Let's talk about this f2f.

[karlklose, 2012-04-25 11:45:43]

PTAL.

http://codereview.chromium.org/10115026/diff/2001/lib/compiler/implementation...
File lib/compiler/implementation/elements/elements.dart (right):

http://codereview.chromium.org/10115026/diff/2001/lib/compiler/implementation...
lib/compiler/implementation/elements/elements.dart:346: cachedType = new
InterfaceType(name, this);
On 2012/04/19 08:20:06, ahe wrote:
> This could be set in the constructor.

Done.

http://codereview.chromium.org/10115026/diff/2001/lib/compiler/implementation...
File lib/compiler/implementation/resolver.dart (right):

http://codereview.chromium.org/10115026/diff/2001/lib/compiler/implementation...
lib/compiler/implementation/resolver.dart:1111: type =
element.computeType(compiler);
As discussed, the check must be on the number of type parameters of the class
element instead. Changed.

[karlklose, 2012-04-30 08:02:41]

Ping.

[ahe, 2012-05-01 17:26:24]

LGTM with a few nits. Sorry for the delay.

Cheers,
Peter

https://chromiumcodereview.appspot.com/10115026/diff/9001/lib/compiler/implem...
File lib/compiler/implementation/resolver.dart (right):

https://chromiumcodereview.appspot.com/10115026/diff/9001/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1108: if (cls.typeParameters.length ==
0) {
&& element.something == 0

https://chromiumcodereview.appspot.com/10115026/diff/9001/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1117:
compiler.internalErrorOnElement(element, "unexpected element kind");
I think you should use the node here.

https://chromiumcodereview.appspot.com/10115026/diff/9001/lib/compiler/implem...
File lib/compiler/implementation/typechecker.dart (right):

https://chromiumcodereview.appspot.com/10115026/diff/9001/lib/compiler/implem...
lib/compiler/implementation/typechecker.dart:67: const InterfaceType(name,
this.element,
I don't think you need the name argument.

[karlklose, 2012-05-02 08:26:50]

Thanks, Peter.

https://chromiumcodereview.appspot.com/10115026/diff/9001/lib/compiler/implem...
File lib/compiler/implementation/resolver.dart (right):

https://chromiumcodereview.appspot.com/10115026/diff/9001/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1108: if (cls.typeParameters.length ==
0) {
Not necessary, as discussed.

https://chromiumcodereview.appspot.com/10115026/diff/9001/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:1117:
compiler.internalErrorOnElement(element, "unexpected element kind");
On 2012/05/01 17:26:24, ahe wrote:
> I think you should use the node here.

Done.

https://chromiumcodereview.appspot.com/10115026/diff/9001/lib/compiler/implem...
File lib/compiler/implementation/typechecker.dart (right):

https://chromiumcodereview.appspot.com/10115026/diff/9001/lib/compiler/implem...
lib/compiler/implementation/typechecker.dart:67: const InterfaceType(name,
this.element,
Yes, removed.