Make sure required type arguments are given when using factories.

Make sure required type arguments are given when using factories.

When a factory is used to create an instance using type arguments and these instances needed the type arguments, we did not track that requirement, because we only knew about the constructor call to the factory.

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

[karlklose, 2013-04-04 14:15:40]

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
File sdk/lib/_internal/compiler/implementation/js_backend/runtime_types.dart
(right):

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
sdk/lib/_internal/compiler/implementation/js_backend/runtime_types.dart:71: if
(universe.usingFactoryWithTypeArguments) {
l. 71-78 are new.

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
sdk/lib/_internal/compiler/implementation/js_backend/runtime_types.dart:80: //
Find all instantiated types that are a subtype of a class that uses
The rest of the method is just copied from the universe version.

[ahe, 2013-04-04 18:43:31]

DBC

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
File sdk/lib/_internal/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
sdk/lib/_internal/compiler/implementation/enqueue.dart:394: void
registerFactoryWithTypeArguments() {
Please pass in the TreeElements so we have a record of who caused this.

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:2544:
compiler.enqueuer.resolution.registerFactoryWithTypeArguments();
s/compiler.enqueuer.resolution/world/

[Johnni Winther, 2013-04-05 08:14:46]

lgtm

[ngeoffray, 2013-04-07 09:38:37]

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
File sdk/lib/_internal/compiler/implementation/js_backend/runtime_types.dart
(right):

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
sdk/lib/_internal/compiler/implementation/js_backend/runtime_types.dart:72: for
(DartType type in universe.instantiatedTypes) {
What do we need to do to avoid this overly pessimistic behavior? Track which
type variables are given to factory constructors? Is that realistic to do?

[karlklose, 2013-04-08 09:45:12]

Thanks for the review!

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
File sdk/lib/_internal/compiler/implementation/enqueue.dart (right):

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
sdk/lib/_internal/compiler/implementation/enqueue.dart:394: void
registerFactoryWithTypeArguments() {
On 2013/04/04 18:43:31, ahe wrote:
> Please pass in the TreeElements so we have a record of who caused this.

Done.

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
File sdk/lib/_internal/compiler/implementation/js_backend/runtime_types.dart
(right):

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
sdk/lib/_internal/compiler/implementation/js_backend/runtime_types.dart:72: for
(DartType type in universe.instantiatedTypes) {
It may be realistic to track the type variables given to factories that directly
call constructors. But as soon as the factory calls a non-static function, the
analysis gets really difficult.

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://chromiumcodereview.appspot.com/13642002/diff/1/sdk/lib/_internal/comp...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:2544:
compiler.enqueuer.resolution.registerFactoryWithTypeArguments();
On 2013/04/04 18:43:31, ahe wrote:
> s/compiler.enqueuer.resolution/world/

Done.

[karlklose, 2013-04-08 10:38:32]

Committed patchset #2 manually as r21057 (presubmit successful).