Check upper bounds of type arguments when allocating objects of a generic type

Check upper bounds of type arguments when allocating objects of a generic type
at run time when they cannot be checked at compile time.
Committed: https://code.google.com/p/dart/source/detail?r=7211

[srdjan, 2012-05-01 23:30:51]

LGTM (you can explain to me in person the one point that I do not understand).

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/tests/vm/src/G...
File runtime/tests/vm/src/GenericTest.dart (left):

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/tests/vm/src/G...
runtime/tests/vm/src/GenericTest.dart:2: // for details. All rights reserved.
Use of this source code is governed by a
2012

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/tests/vm/src/G...
runtime/tests/vm/src/GenericTest.dart:4: // VMOptions=--enable_type_checks
--enable_asserts
You don't need asserts, do you?

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/tests/vm/src/G...
runtime/tests/vm/src/GenericTest.dart:10: }
Would it be good to have a similar test in the shared code (just do not test
details of TypeError)?

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/vm/code_genera...
File runtime/vm/code_generator.cc (right):

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/vm/code_genera...
runtime/vm/code_generator.cc:197: // Return value: newly allocated object.
Can you measure if there is any performance impact of the correct allocation
with bounds (e.g., running dart2js)

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/vm/object.cc#n...
runtime/vm/object.cc:2207: (Index() == other.Index());
I do not understand this change.

[regis, 2012-05-02 01:34:16]

Thanks!

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/tests/vm/src/G...
File runtime/tests/vm/src/GenericTest.dart (left):

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/tests/vm/src/G...
runtime/tests/vm/src/GenericTest.dart:2: // for details. All rights reserved.
Use of this source code is governed by a
On 2012/05/01 23:30:51, srdjan wrote:
> 2012

This test is being deleted. There is an almost identical test now called
generic_test.dart.

The part of this test that was VM specific is not relevant anymore, because the
fields srcType, dstType, and dstName of a TypeError are not set for malformed
type errors and failed bound checks.

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/tests/vm/src/G...
runtime/tests/vm/src/GenericTest.dart:4: // VMOptions=--enable_type_checks
--enable_asserts
On 2012/05/01 23:30:51, srdjan wrote:
> You don't need asserts, do you?

No.

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/tests/vm/src/G...
runtime/tests/vm/src/GenericTest.dart:10: }
On 2012/05/01 23:30:51, srdjan wrote:
> Would it be good to have a similar test in the shared code (just do not test
> details of TypeError)?

We have one.

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/vm/code_genera...
File runtime/vm/code_generator.cc (right):

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/vm/code_genera...
runtime/vm/code_generator.cc:197: // Return value: newly allocated object.
On 2012/05/01 23:30:51, srdjan wrote:
> Can you measure if there is any performance impact of the correct allocation
> with bounds (e.g., running dart2js)

Yes, I will measure (I'll check with you for the procedure), but we do not
really have the choice. Of course, type propagation will help here as well.

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/10280007/diff/1/runtime/vm/object.cc#n...
runtime/vm/object.cc:2207: (Index() == other.Index());
On 2012/05/01 23:30:51, srdjan wrote:
> I do not understand this change.

The previous test was too lenient. You can imagine comparing two signatures, one
returning K, the other one returning V, both type parameters. They should
differ.

Previously, we only encountered type parameters when testing signatures,
because, beside uninstantiated function result types and uninstantiated function
parameter types, we only tested instantiated types. With this change, we can
also test uninstantiated types. This is how we decide whether a run time bounds
check is necessary or not. We perform a subtype check of the uninstantiated
bounds and only return true if the test is guaranteed to succeed.
For example, with class A<K, V extends K>, new A<T, T> called from with a class
<T> will never require a run time bounds check, even it T is uninstantiated at
compile time.

I have added a comment.