classpath-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [cp-patches] [generics] Enum#valueOf implementation


From: Ewout Prangsma
Subject: Re: [cp-patches] [generics] Enum#valueOf implementation
Date: Wed, 03 Aug 2005 19:02:29 +0200
User-agent: Mozilla Thunderbird 1.0.6 (Windows/20050716)

Hi Andrew,

I've tested this patch in JNode, where is works without any problems.

Ewout

Andrew John Hughes wrote:
On Mon, 2005-07-25 at 12:38 +0200, Mark Wielaard wrote:
  
On Mon, 2005-07-25 at 00:33 -0600, Tom Tromey wrote:
    
"Ewout" == Ewout Prangsma <address@hidden> writes:
                
Ewout> Here's the unified diff.  Again my question if someone can
Ewout> commit this, or give me CVS access rights.

It still needs a ChangeLog entry.
Also the code is not formatted according to the Classpath style.
      
Here is an example of a correct ChangeLog entry and the patch formatted
as (I also added a little documentation, please check).

2005-07-25  Ewout Prangsma  <address@hidden>

        * java/lang/Enum.java (valueOf): implemented.

ChangeLog and Coding style is discussed int he hacking guide:
http://www.gnu.org/software/classpath/docs/hacking.html#SEC6
http://www.gnu.org/software/classpath/docs/hacking.html#SEC8

One question. Will the IllegalAccessException really never be thrown? I
can imagine that the Enum value fields are always public or package
private to java.lang in which case this is true (but I haven't studied
Enums at all). Otherwise we might need a PrivilegedAction that calls
setAccessible(true).

Andrew, could you please take a look and tell me whether it is OK to
commit (compiles, don't have ecj or gcjx installed here) and whether or
not it interferes with you current merging work?

Thanks,

Mark
    

Sorry for my delay in replying; I only just spotted this, thanks to
Mark.  The patch compiles here, or, at least, it appears to.  Currently,
what appears to be a bug in ecj
(https://bugs.eclipse.org/bugs/show_bug.cgi?id=105531) is preventing me
from doing a full compile, but no other errors cropped up from this
patch.

Enums are (usually) compiler-generated, so the fields should always be
public, unless someone is doing something pretty bizarre (and in which
case, they probably deserve an error).  The enums use an extension of
Java syntax akin to the C syntax:

enum Color
{ 
	red, 
	green, 
	blue 
};

which generates a normal Java class, extending the Enum class.

class TestEnums.Color extends java.lang.Enum {
    /* ACC_SUPER bit NOT set */
    public static final TestEnums.Color red;
    public static final TestEnums.Color green;
    public static final TestEnums.Color blue;
    public static final long num;
    static {};
    public static final TestEnums.Color[] values();
    public static final TestEnums.Color valueOf(java.lang.String);
}

So, in this case, you'd call Enum.valueOf(TestEnums.Color.class, "red")
for example.  The signature for the method ensures that the input will
be a enum derivative.

So, as far as I can see, the patch is okay, and now would be a good time
to get it in, given the recent merge.   Unfortunately, I obviously
haven't been able to test it in use because I can't obtain a successful
build at present.

Cheers,
  

_______________________________________________ Classpath-patches mailing list address@hidden http://lists.gnu.org/mailman/listinfo/classpath-patches

reply via email to

[Prev in Thread] Current Thread [Next in Thread]