[cp-patches] Patch: Reduce number of compilation warnings
Hi, the attached patch reduces the number of warnings emitted when compiling classpath with jikes (in particular as part of the Jikes RVM build process). The warnings were: 1) by a negative amount - this was in the code to achieve a rotate. To make the code readable the patch adds a rotate method which is called instead. As the rotate method is private it should be an excellent candidate for inlining and not impact on performance. 2) final fields that could have been static weren't - so I made them static :-) 3) reference of a static field from an instance rather than from the class - made to reference the class instead 4) the inner class EditorContainer of DefaultTreeCellEditor had a method whose name matched that of being constructor but it wasn't a constructor - the comments in the code suggested this method shouldn't be there, so I removed it The only remaining warning was with ./gnu/java/rmi/registry/RegistryImpl_Stub.java that contains private methods whose names begin with $ and that could possibly (but quite unlikely) create a naming conflict with an automatically generated class/field. This class is automatically generated by rmic, and I couldn't see in rmic where the $ signs were creeping in :-( The patch is just a suggestion, but it cleans up watching a build for me, which could potentially help spot new bugs in the future. Regards, Ian Rogers -- http://www.cs.man.ac.uk/~irogers Index: external/sax/org/xml/sax/helpers/ParserAdapter.java === RCS file: /sources/classpath/classpath/external/sax/org/xml/sax/helpers/ParserAdapter.java,v retrieving revision 1.1 diff -u -r1.1 ParserAdapter.java --- external/sax/org/xml/sax/helpers/ParserAdapter.java 23 Dec 2004 22:38:42 - 1.1 +++ external/sax/org/xml/sax/helpers/ParserAdapter.java 13 Apr 2006 15:23:57 - @@ -561,7 +561,7 @@ // note funky case: localname can be null // when declaring the default prefix, and // yet the uri isn't null. - atts.addAttribute (nsSupport.XMLNS, prefix, + atts.addAttribute (NamespaceSupport.XMLNS, prefix, attQName.intern(), type, value); else atts.addAttribute (, , Index: gnu/java/security/hash/MD4.java === RCS file: /sources/classpath/classpath/gnu/java/security/hash/MD4.java,v retrieving revision 1.1 diff -u -r1.1 MD4.java --- gnu/java/security/hash/MD4.java 26 Jan 2006 02:25:10 - 1.1 +++ gnu/java/security/hash/MD4.java 13 Apr 2006 15:23:58 - @@ -222,107 +222,115 @@ dd = d; aa += ((bb cc) | ((~bb) dd)) + X0; -aa = aa 3 | aa -3; +aa = rotate(aa, 3); dd += ((aa bb) | ((~aa) cc)) + X1; -dd = dd 7 | dd -7; +dd = rotate(dd, 7); cc += ((dd aa) | ((~dd) bb)) + X2; -cc = cc 11 | cc -11; +cc = rotate(cc, 11); bb += ((cc dd) | ((~cc) aa)) + X3; -bb = bb 19 | bb -19; +bb = rotate(bb, 19); aa += ((bb cc) | ((~bb) dd)) + X4; -aa = aa 3 | aa -3; +aa = rotate(aa, 3); dd += ((aa bb) | ((~aa) cc)) + X5; -dd = dd 7 | dd -7; +dd = rotate(dd, 7); cc += ((dd aa) | ((~dd) bb)) + X6; -cc = cc 11 | cc -11; +cc = rotate(cc, 11); bb += ((cc dd) | ((~cc) aa)) + X7; -bb = bb 19 | bb -19; +bb = rotate(bb, 19); aa += ((bb cc) | ((~bb) dd)) + X8; -aa = aa 3 | aa -3; +aa = rotate(aa, 3); dd += ((aa bb) | ((~aa) cc)) + X9; -dd = dd 7 | dd -7; +dd = rotate(dd, 7); cc += ((dd aa) | ((~dd) bb)) + X10; -cc = cc 11 | cc -11; +cc = rotate(cc, 11); bb += ((cc dd) | ((~cc) aa)) + X11; -bb = bb 19 | bb -19; +bb = rotate(bb, 19); aa += ((bb cc) | ((~bb) dd)) + X12; -aa = aa 3 | aa -3; +aa = rotate(aa, 3); dd += ((aa bb) | ((~aa) cc)) + X13; -dd = dd 7 | dd -7; +dd = rotate(dd, 7); cc += ((dd aa) | ((~dd) bb)) + X14; -cc = cc 11 | cc -11; +cc = rotate(cc, 11); bb += ((cc dd) | ((~cc) aa)) + X15; -bb = bb 19 | bb -19; +bb = rotate(bb, 19); aa += ((bb (cc | dd)) | (cc dd)) + X0 + 0x5a827999; -aa = aa 3 | aa -3; +aa = rotate(aa, 3); dd += ((aa (bb | cc)) | (bb cc)) + X4 + 0x5a827999; -dd = dd 5 | dd -5; +dd = rotate(dd, 5); cc += ((dd (aa | bb)) | (aa bb)) + X8 + 0x5a827999; -cc = cc 9 | cc -9; +cc = rotate(cc, 9); bb += ((cc (dd | aa)) | (dd aa)) + X12 + 0x5a827999; -bb = bb 13 | bb -13; +bb = rotate(bb, 13); aa += ((bb (cc | dd)) | (cc dd)) + X1 + 0x5a827999; -aa = aa 3 | aa -3; +aa = rotate(aa, 3); dd += ((aa (bb | cc)) | (bb cc)) + X5 + 0x5a827999; -dd = dd 5 | dd -5; +dd = rotate(dd, 5); cc += ((dd (aa | bb)) | (aa bb)) + X9 + 0x5a827999; -cc = cc 9 | cc -9; +cc = rotate(cc, 9); bb += ((cc (dd | aa)) | (dd aa)) + X13 + 0x5a827999; -bb =
Re: [cp-patches] Patch: Reduce number of compilation warnings
Ian == Ian Rogers [EMAIL PROTECTED] writes:
Ian 1) by a negative amount - this was in the code to achieve a
Ianrotate. To make the code readable the patch adds a rotate
Ianmethod which is called instead. As the rotate method is private
Ianit should be an excellent candidate for inlining and not impact
Ianon performance.
This one is fine by me. However I think some of the other changes
are problematic :-(
Ian 2) final fields that could have been static weren't - so I made them
Ianstatic :-)
Usually this is done for API compatibility and represents a historical
accident. At least, that's true for public API. We can't change
this.
If jikes warnings for this kind of thing are annoying, about all you
can do is turn them off.
Ian 4) the inner class EditorContainer of DefaultTreeCellEditor had a
Ianmethod whose name matched that of being constructor but it wasn't a
Ianconstructor - the comments in the code suggested this method
Ianshouldn't be there, so I removed it
Required for API compatibility.
Ian Index: external/sax/org/xml/sax/helpers/ParserAdapter.java
As a rule we don't touch external/. Instead try upstream... though
I've had no results so far with this.
Ian + private static int rotate(int x, int y) {
Ian + return (x y) | (x -y);
Ian + }
Coding style... open brace goes on its own line.
Tom
Re: [cp-patches] Patch: Reduce number of compilation warnings
Thanks Tom,
I thought I'd fix all the warnings and wait for people to complain. I
agree with what you say and I've attached a fixed patch for just MD4 and
MD5, any idea on how to stop the $ symbols being generated by rmic? They
are on private fields so altering the names shouldn't matter.
Ian
Tom Tromey wrote:
Ian + private static int rotate(int x, int y) {
Ian +return (x y) | (x -y);
Ian + }
Coding style... open brace goes on its own line.
Tom
Index: gnu/java/security/hash/MD4.java
===
RCS file: /sources/classpath/classpath/gnu/java/security/hash/MD4.java,v
retrieving revision 1.1
diff -u -r1.1 MD4.java
--- gnu/java/security/hash/MD4.java 26 Jan 2006 02:25:10 - 1.1
+++ gnu/java/security/hash/MD4.java 13 Apr 2006 18:59:37 -
@@ -222,107 +222,116 @@
dd = d;
aa += ((bb cc) | ((~bb) dd)) + X0;
-aa = aa 3 | aa -3;
+aa = rotate(aa, 3);
dd += ((aa bb) | ((~aa) cc)) + X1;
-dd = dd 7 | dd -7;
+dd = rotate(dd, 7);
cc += ((dd aa) | ((~dd) bb)) + X2;
-cc = cc 11 | cc -11;
+cc = rotate(cc, 11);
bb += ((cc dd) | ((~cc) aa)) + X3;
-bb = bb 19 | bb -19;
+bb = rotate(bb, 19);
aa += ((bb cc) | ((~bb) dd)) + X4;
-aa = aa 3 | aa -3;
+aa = rotate(aa, 3);
dd += ((aa bb) | ((~aa) cc)) + X5;
-dd = dd 7 | dd -7;
+dd = rotate(dd, 7);
cc += ((dd aa) | ((~dd) bb)) + X6;
-cc = cc 11 | cc -11;
+cc = rotate(cc, 11);
bb += ((cc dd) | ((~cc) aa)) + X7;
-bb = bb 19 | bb -19;
+bb = rotate(bb, 19);
aa += ((bb cc) | ((~bb) dd)) + X8;
-aa = aa 3 | aa -3;
+aa = rotate(aa, 3);
dd += ((aa bb) | ((~aa) cc)) + X9;
-dd = dd 7 | dd -7;
+dd = rotate(dd, 7);
cc += ((dd aa) | ((~dd) bb)) + X10;
-cc = cc 11 | cc -11;
+cc = rotate(cc, 11);
bb += ((cc dd) | ((~cc) aa)) + X11;
-bb = bb 19 | bb -19;
+bb = rotate(bb, 19);
aa += ((bb cc) | ((~bb) dd)) + X12;
-aa = aa 3 | aa -3;
+aa = rotate(aa, 3);
dd += ((aa bb) | ((~aa) cc)) + X13;
-dd = dd 7 | dd -7;
+dd = rotate(dd, 7);
cc += ((dd aa) | ((~dd) bb)) + X14;
-cc = cc 11 | cc -11;
+cc = rotate(cc, 11);
bb += ((cc dd) | ((~cc) aa)) + X15;
-bb = bb 19 | bb -19;
+bb = rotate(bb, 19);
aa += ((bb (cc | dd)) | (cc dd)) + X0 + 0x5a827999;
-aa = aa 3 | aa -3;
+aa = rotate(aa, 3);
dd += ((aa (bb | cc)) | (bb cc)) + X4 + 0x5a827999;
-dd = dd 5 | dd -5;
+dd = rotate(dd, 5);
cc += ((dd (aa | bb)) | (aa bb)) + X8 + 0x5a827999;
-cc = cc 9 | cc -9;
+cc = rotate(cc, 9);
bb += ((cc (dd | aa)) | (dd aa)) + X12 + 0x5a827999;
-bb = bb 13 | bb -13;
+bb = rotate(bb, 13);
aa += ((bb (cc | dd)) | (cc dd)) + X1 + 0x5a827999;
-aa = aa 3 | aa -3;
+aa = rotate(aa, 3);
dd += ((aa (bb | cc)) | (bb cc)) + X5 + 0x5a827999;
-dd = dd 5 | dd -5;
+dd = rotate(dd, 5);
cc += ((dd (aa | bb)) | (aa bb)) + X9 + 0x5a827999;
-cc = cc 9 | cc -9;
+cc = rotate(cc, 9);
bb += ((cc (dd | aa)) | (dd aa)) + X13 + 0x5a827999;
-bb = bb 13 | bb -13;
+bb = rotate(bb, 13);
aa += ((bb (cc | dd)) | (cc dd)) + X2 + 0x5a827999;
-aa = aa 3 | aa -3;
+aa = rotate(aa, 3);
dd += ((aa (bb | cc)) | (bb cc)) + X6 + 0x5a827999;
-dd = dd 5 | dd -5;
+dd = rotate(dd, 5);
cc += ((dd (aa | bb)) | (aa bb)) + X10 + 0x5a827999;
-cc = cc 9 | cc -9;
+cc = rotate(cc, 9);
bb += ((cc (dd | aa)) | (dd aa)) + X14 + 0x5a827999;
-bb = bb 13 | bb -13;
+bb = rotate(bb, 13);
aa += ((bb (cc | dd)) | (cc dd)) + X3 + 0x5a827999;
-aa = aa 3 | aa -3;
+aa = rotate(aa, 3);
dd += ((aa (bb | cc)) | (bb cc)) + X7 + 0x5a827999;
-dd = dd 5 | dd -5;
+dd = rotate(dd, 5);
cc += ((dd (aa | bb)) | (aa bb)) + X11 + 0x5a827999;
-cc = cc 9 | cc -9;
+cc = rotate(cc, 9);
bb += ((cc (dd | aa)) | (dd aa)) + X15 + 0x5a827999;
-bb = bb 13 | bb -13;
+bb = rotate(bb, 13);
aa += (bb ^ cc ^ dd) + X0 + 0x6ed9eba1;
-aa = aa 3 | aa -3;
+aa = rotate(aa, 3);
dd += (aa ^ bb ^ cc) + X8 + 0x6ed9eba1;
-dd = dd 9 | dd -9;
+dd = rotate(dd, 9);
cc += (dd ^ aa ^ bb) + X4 + 0x6ed9eba1;
-cc = cc 11 | cc -11;
+cc = rotate(cc, 11);
bb += (cc ^ dd ^ aa) + X12 + 0x6ed9eba1;
-bb = bb 15 | bb -15;
+bb = rotate(bb, 15);
aa += (bb ^ cc ^ dd) + X2 + 0x6ed9eba1;
-aa = aa 3 | aa -3;
+aa = rotate(aa, 3);
dd += (aa ^ bb ^ cc) + X10 + 0x6ed9eba1;
-dd = dd 9 | dd -9;
+dd = rotate(dd, 9);
cc += (dd ^ aa ^ bb) + X6 + 0x6ed9eba1;
-cc = cc 11 | cc -11;
+cc = rotate(cc, 11);
bb += (cc ^ dd ^ aa) + X14 + 0x6ed9eba1;
-bb = bb 15 | bb -15;
+bb = rotate(bb, 15);
aa += (bb ^ cc ^ dd) + X1 +
Re: [cp-patches] Patch: Reduce number of compilation warnings
Hi Audrius, Wouldn't it be better to regenerate the sources rather than manually remove the $ signs? I don't know how the RMI compiler generated the sources. I'm happy to modify the files by hand otherwise. Thanks! Ian Audrius Meskauskas wrote: any idea on how to stop the $ symbols being generated by rmic? They are on private fields so altering the names shouldn't matter. I would say, just rename these private fields to match our coding standard. I see no reason to use the reserved characters there. This class is not generated during the each build and can be edited like any other. Our RMI source code generator in RMIC package is currently configured to follow the standard Classpath naming rules and would not use the dollar sign for the names of the private fields. Regards Audrius.
Re: [cp-patches] Patch: Reduce number of compilation warnings
On Apr 13, 2006, at 8:52 AM, Ian Rogers wrote: Hi, the attached patch reduces the number of warnings emitted when compiling classpath with jikes (in particular as part of the Jikes RVM build process). The warnings were: 1) by a negative amount - this was in the code to achieve a rotate. To make the code readable the patch adds a rotate method which is called instead. As the rotate method is private it should be an excellent candidate for inlining and not impact on performance. I'd suggest (besides complaining to the jikes maintainers (crickets...) that this warning is useless and should be removed) instead just replacing `-n' with `(32-n)'. Of course, I usually just compile with `JIKES=jikes -nowarn' anyway, because the warnings jikes started emitting are often useless.
