[request for review] 4877954: RFE: Special syntax for core interfaces

[Dmytro Sheyko]

Added -L switch


From: dmytro_sheyko at hotmail.comTo: compiler-dev at openjdk.java.netSubject: RE: [request for review] 4877954: RFE: Special syntax for core interfacesDate: Thu, 22 Nov 2007 23:39:15 +0700


Hi, I am presenting new version of the patch, where I was trying to take into account all remarks. Well, see my answer below.In Lower.visitBinary(), acmp is used two times with different meanings,import static and local variable, i think the local variable could be renamed. done. Futhermore, unboxedComparison can be inlined in visiBinary() because it is used only once and the if/else will be more balanced. done. The most obvious error seems to be in ConstFold, where you are comparing the string value of the constants, which seems unlikely to yield the correct result. done. New static method stringValue (like intValue, doubleValue and so on) was added and used. In Lower.visitBinary, it seems wrong to drop this code in at the head of the method. If nothing else, I would think you should translate the lhs and rhs first. I think we shouldn't; lhs and rhs are to be translated recursively after reducing "comparable binary" to "ordinary binary". If we translate lhs and rhs first, they will be translated twice. However, in order to make code a little bit clearer I have divided Lower.visitBinary into visitOrdinaryBinary and visitComparableBinary. Then in line 2785, the asymmetry of taking the primitiveType from then lhs seems incorrect, unless you can demonstrate that the lhs and rhs will have the same type. added assertion. In the tests, you use a relative small set of expressions on either side of the comparables -- for example, I didn't see any expressions either side (other that variables and function calls). added test NestedExpr, however you comments are still welcome how to make test set more complete. Separately, Christian Plesner Hansen is proposing a standardized way of enabling language extensions based on a new -L switch. That seems like a good idea, and it would be nice to standardize on that, rather than using the -XD space which is more for the hidden options we don't really want users to know about :-) not now. I think it has to be separate patch/commit specially for -L switch.

Get news, entertainment and everything you care about at Live.com. Check it out! 
_________________________________________________________________
News, entertainment and everything you care about at Live.com. Get it now!
http://www.live.com/getstarted.aspx
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20071127/31381ab8/attachment.html 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: RFE4877954-comp.diff
Url: http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20071127/31381ab8/RFE4877954-comp.diff 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tests.jar
Type: application/x-zip-compressed
Size: 9141 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20071127/31381ab8/tests.jar