RFR (M): Generate checks for vbox/vunbox; extend CI to include VCC-DVT mapping

[Zoltán Majó]

Hi Tobias,


thank you for the feedback.

On 04/03/2017 03:12 PM, Tobias Hartmann wrote:
> [...]
>
>>> - Since we don't support subtyping for value types, shouldn't we check for type equality instead of a subtype relation?
>>>     - In vbox(), you can get the exact type of the ValueTypeNode via ValueTypeNode::value_klass() and you can check that for equality with the target_dvt_klass
>> I agree -- let's do that.
> Looks good. I just wonder if we really need 'guarantee' for the static checks. Existing code uses 'assert' for similar checks but it doesn't matter too much for now.

OK, let's keep the guarantees for now and change them to asserts later 
on if needed.

>
>>>     - In vunbox(), I would check for equality of the source_type and the target_vcc_klass
>> In some cases (e.g., when the declared type source_type is java.lang.Object) the check you mention is not sufficient. We need to generate a dynamic type check in that case -- test_64 covers that scenario.
> Sure but what I meant is a dynamic check for type equality of the VCC (derived from the target value type) and the source type instead of a subtype check.

Oh, I see. The code in webrev.01 generates code to perform a type 
equality check (and does not generate subtype checks).

> Because we don't support subtyping, right?

That is right.

> Test64 should only ever pass if 'vcc' is ValueCapableClass1.

That is correct and that's what also happens.

>
> For example, with VCC A and class B which is a subtype of A, vunbox of B to A$Value should fail, right? I think we should also add a test for this case.

That is also correct. Unfortunately, we cannot add a test like that 
because VCCs must be final (I tried making ValueCapableClass1 but that 
causes an java.lang.InternalError to be thrown "DeriveValueType class 
'compiler.valhalla.valuetypes.ValueCapableClass1' is not a final class").

Thank you!

Best regards,


Zoltan

> Thanks,
> Tobias