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

[Dmytro Sheyko]

Thank you for your comments Rémi,see my answer below



> I don't see why you want bypass a possible bridge,> i.e. lhs.compareTo((Object) rhs) instead of lhs.compareTo(erasure(rhs)).
This was done in order not to choose wrong 'compareTo' method.
Let's look at following sample
 
class A implements Comparable<A> {
    public int compareTo(A a) { ... }
    public int compareTo(B b) { ... }
}
class B extends A {...}
 
B x, y;
x < y; // here we have to choose either compareTo(A) or compareTo(Object), but not compareTo(B)
 
Class A has 3 'compareTo' methods:
1. compareTo(A) - genuine comparable method
2. compareTo(Object) - bridge to compareTo(A)
3. compareTo(B) - just overloaded, it pretends to be comparable but it's not
 
Current implementation chooses bridge method (in my opinion it is simplest correct implementation for this case), and I agree we  should prefer to choose 'genuine comparable method' rather than bridge.
 
> > 2. Expressions of boolean type (i.e. boolean or Boolean) are considered> > comparable so that 'false' is less than 'true'. Both operands are > > evaluated> > always.> It seams that Boolean implement Comparable<Boolean> since 1.5,> so i assume it's the same semantic.
 
Exactly! I had doubts as to permitting relational operators for booleans; and finally I've decided that it would be more natural to permit them (of course, according to Boolean.compareTo()) than not to do so.
> > Attached svn diff. > About your patch:> - i don't like the fact the compiler is able to perform constant > folding on compareTo.> Enum#ordinal() or Enum#name() are not evaluated as constant, i don't see> why compareTo is different.
 
Constant folding was performed for numeric comparison before my change, so it seems to me that constant folding should be performed for strings and booleans as well. Why not? BTW, Constant folding is not performed for enums for now.
 
> minor comment, why in Attr you change tree.lhs.pos() to tree.pos() ?
Error message "operator.cant.be.applied" is reported from two places:
1. Resolve.java:1639 (report) - if proper operator has not been found, it says that error is at operator position;
2. Check.java:1967 (checkOperator) - if proper operator has been found but it is marked as error, it says that error is at lhs position;
I just made error reporting in the second place consistent with the first place. It is not so essential to human, but for tests it is necessary to have predictable and stable error messages.
> - introducing all big*Type in Symtab is an error.> they should not be stored in the table like that, they are store as > Name and> not Type (search the comment "initialize builtin types" in Symtab.java)> and unlike your solution are loaded using classreader,> take a look to Types.boxedClass() and Types.unboxedType().> > So i think Lower.visitBinary should be rewrite using Types.unbox() in > order to:> works with type parameters and wildcards and reuse the current Symtab> infrastructure that deals with boxing/unboxing (Symtab.boxedName).
Ok, I will think about it.
 
_________________________________________________________________
Explore the seven wonders of the world
http://search.msn.com/results.aspx?q=7+wonders+world&mkt=en-US&form=QBRE
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20071113/a33ae64f/attachment.html