RFR (M): Generate checks for vbox/vunbox; extend CI to include VCC-DVT mapping
tobias.hartmann at oracle.com
Mon Apr 3 14:23:20 UTC 2017
On 03.04.2017 16:08, Zoltán Majó wrote:
>> 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.
Okay, sounds good.
>>>> - 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).
Right, I got confused by the 'target_vcc_klass->is_subclass_of(source_type->klass()' check.
>> 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.
Okay, thanks for verifying.
>> 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").
Okay, if this ever changes we can still add a test.
Your changes look good to me!
More information about the valhalla-dev