[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-mlvm-dev
Subject:    Re: RFR (L) 8037210: Get rid of char-based descriptions 'J' of basic types
From:       Vladimir Ivanov <vladimir.x.ivanov () oracle ! com>
Date:       2014-03-26 15:01:41
Message-ID: 5332EBD5.8090100 () oracle ! com
[Download RAW message or body]

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). 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@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@oracle.com <mailto:vladimir.x.ivanov@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@openjdk.java.net
> > > > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
> > > > 
> > _______________________________________________
> > mlvm-dev mailing list
> > mlvm-dev@openjdk.java.net
> > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
> 
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev@openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
> 
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev


[prev in list] [next in list] [prev in thread] [next in thread]