Mark the local for this as used if a constructor call needs type variables.

Mark the local for this as used if a constructor call needs type variables.

R=floitsch@google.com
BUG=http://code.google.com/p/dart/issues/detail?id=4829

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

[ngeoffray, 2012-09-03 12:57:23]

Also add the test case?

http://codereview.chromium.org/10915054/diff/1/lib/compiler/implementation/cl...
File lib/compiler/implementation/closure.dart (right):

http://codereview.chromium.org/10915054/diff/1/lib/compiler/implementation/cl...
lib/compiler/implementation/closure.dart:337: // TODO(karlklose): factories
cannot get the type argument via this.
The resolve must have checked this already. Either assert or remove the TODO.

http://codereview.chromium.org/10915054/diff/1/lib/compiler/implementation/ss...
File lib/compiler/implementation/ssa/builder.dart (right):

http://codereview.chromium.org/10915054/diff/1/lib/compiler/implementation/ss...
lib/compiler/implementation/ssa/builder.dart:2473: HInstruction thisValue = new
HThis();
I don't think this is right, you should use [thisInstruction]

[floitsch, 2012-09-03 13:23:01]

LGTM once Nicolas' comments have been addressed.

[karlklose, 2012-09-03 13:55:19]

PTAL.

https://chromiumcodereview.appspot.com/10915054/diff/1/lib/compiler/implement...
File lib/compiler/implementation/closure.dart (right):

https://chromiumcodereview.appspot.com/10915054/diff/1/lib/compiler/implement...
lib/compiler/implementation/closure.dart:337: // TODO(karlklose): factories
cannot get the type argument via this.
On 2012/09/03 12:57:23, ngeoffray wrote:
> The resolve must have checked this already. Either assert or remove the TODO.

Done.

https://chromiumcodereview.appspot.com/10915054/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10915054/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:2473: HInstruction thisValue = new
HThis();
This was actually unnecessary. Removed.

[ngeoffray, 2012-09-03 14:03:36]

LGTM

https://chromiumcodereview.appspot.com/10915054/diff/4001/tests/language/clos...
File tests/language/closure_type_variables_test.dart (right):

https://chromiumcodereview.appspot.com/10915054/diff/4001/tests/language/clos...
tests/language/closure_type_variables_test.dart:5: // Test that type variables
are available in closures.
Maybe also add corner cases like doing it in a constructor/factory/static
methods.

[floitsch, 2012-09-03 14:04:15]

LGTM.

https://chromiumcodereview.appspot.com/10915054/diff/4001/lib/compiler/implem...
File lib/compiler/implementation/closure.dart (right):

https://chromiumcodereview.appspot.com/10915054/diff/4001/lib/compiler/implem...
lib/compiler/implementation/closure.dart:337: if (closureData.thisElement !==
null) {
Add comment why this is necessary. Maybe find a more direct way of testing it?
for example: closureData.isInsideInstanceMethod