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

[Dmytro Sheyko]

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.
_________________________________________________________________
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/20071122/d324c43b/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/20071122/d324c43b/RFE4877954-comp.diff 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tests.jar
Type: application/x-zip-compressed
Size: 9409 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20071122/d324c43b/tests.jar