RFR (L) 8037210: Get rid of char-based descriptions 'J' of basic types

Vladimir Ivanov vladimir.x.ivanov at oracle.com
Tue Apr 8 09:20:11 UTC 2014


Chris, sorry for the late reply.

 >> Here's a version with the new naming scheme:
>> http://cr.openjdk.java.net/~vlivanov/8037210/webrev.03.naming
>>
>> I like existing naming scheme and OBJECT/VOID/LONG/etc names are quite popular(e.g. Wrapper & ASM (Opcodes) use them).
>
> Of course they are popular because these are the type names.  There is no type L; it’s an object.  I don’t understand why we have to use different names just because they are used in other namespaces.  This is not a C define.
I see 2 problems with the naming scheme you propose.

(1) Wrapper, Opcodes & BasicType collide in some places. If element 
names are the same, static import doesn't work and all usages should be 
disambiguated.

(2) BasicType.I_TYPE corresponds to 5 primitive types. It's misleading 
to call it BasicType.INT (or BasicType.INTEGER) (consider Wrapper.INT vs 
BasicType.INT).

Current naming scheme makes correspondence with JVM basic types explicit.

Best regards,
Vladimir Ivanov

>
>> So, I'm in favor of leaving it as is.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> On 3/26/14 12:24 AM, Christian Thalinger wrote:
>>> +     enum BasicType {
>>> +         L_TYPE('L', Object.class, Wrapper.OBJECT),  // all reference types
>>> +         I_TYPE('I', int.class,    Wrapper.INT),
>>> +         J_TYPE('J', long.class,   Wrapper.LONG),
>>> +         F_TYPE('F', float.class,  Wrapper.FLOAT),
>>> +         D_TYPE('D', double.class, Wrapper.DOUBLE),  // all primitive types
>>> +         V_TYPE('V', void.class,   Wrapper.VOID);    // not valid in all contexts
>>>
>>> I would suggest to not name them X_TYPE but give them meaningful names like Int, Float, Void.  The enum BasicType already infers that it’s a type.  If you think it’s not clear that it’s a type just use BasicType.Double in that places.
>>>
>>> On Mar 24, 2014, at 9:29 AM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
>>>
>>>> Updated version:
>>>> http://cr.openjdk.java.net/~vlivanov/8037210/webrev.03/
>>>>
>>>> - changed the way how arrays of types are created:
>>>>         static final BasicType[] ALL_TYPES = BasicType.values();
>>>>         static final BasicType[] ARG_TYPES = Arrays.copyOf(ALL_TYPES, ALL_TYPES.length-1);
>>>>
>>>> - added a test for BasicType (LambdaFormTest.testBasicType).
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>> On 3/22/14 2:08 AM, Vladimir Ivanov wrote:
>>>>> John, thanks for the feedback.
>>>>>
>>>>> Updated webrev:
>>>>> http://cr.openjdk.java.net/~vlivanov/8037210/webrev.02
>>>>>
>>>>> Also moved LambdaForm.testShortenSignature() into a stand-alone unit test.
>>>>>
>>>>> Best regards,
>>>>> Vladimir Ivanov
>>>>>
>>>>> On 3/21/14 10:54 PM, John Rose wrote:
>>>>>> On Mar 21, 2014, at 8:49 AM, Vladimir Ivanov
>>>>>> <vladimir.x.ivanov at oracle.com <mailto:vladimir.x.ivanov at oracle.com>>
>>>>>> wrote:
>>>>>>
>>>>>>> Thanks for the feedback.
>>>>>>>
>>>>>>> What do you think about the following:
>>>>>>> http://cr.openjdk.java.net/~vlivanov/8037210/webrev.01/
>>>>>>
>>>>>> That looks nice.  Strong typing; who woulda' thunk it.  :-)
>>>>>>
>>>>>> The uses of ".ordinal()" are the extra cost relative to using just small
>>>>>> integers.  They seem totally reasonable in the code.
>>>>>>
>>>>>> I suggest either wrapping "ordinal" as something like "id" or else
>>>>>> changing temp names like "id" to "ord", to reinforce the use of a common
>>>>>> name.
>>>>>>
>>>>>> Don't make the enum class public.  And especially don't make the mutable
>>>>>> arrays public:
>>>>>>
>>>>>> +        public static final BasicType[] ALL_TYPES = { L_TYPE, I_TYPE,
>>>>>> J_TYPE, F_TYPE, D_TYPE, V_TYPE };
>>>>>> +        public static final BasicType[] ARG_TYPES = { L_TYPE, I_TYPE,
>>>>>> J_TYPE, F_TYPE, D_TYPE };
>>>>>>
>>>>>> — John
>>>>>>
>>>>>> P.S.  That would only be safe if we had (what we don't yet) a notion of
>>>>>> frozen arrays like:
>>>>>>
>>>>>> +        public static final BasicType final[] ALL_TYPES = { L_TYPE,
>>>>>> I_TYPE, J_TYPE, F_TYPE, D_TYPE, V_TYPE };
>>>>>> +        public static final BasicType final[] ARG_TYPES = { L_TYPE,
>>>>>> I_TYPE, J_TYPE, F_TYPE, D_TYPE };
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> mlvm-dev mailing list
>>>>>> mlvm-dev at openjdk.java.net
>>>>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>>>>>>
>>>> _______________________________________________
>>>> mlvm-dev mailing list
>>>> mlvm-dev at openjdk.java.net
>>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>>>
>>> _______________________________________________
>>> mlvm-dev mailing list
>>> mlvm-dev at openjdk.java.net
>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>>>
>> _______________________________________________
>> mlvm-dev mailing list
>> mlvm-dev at openjdk.java.net
>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>



More information about the core-libs-dev mailing list