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

[Rémi Forax]

Dmytro Sheyko a écrit :
> Hi,
Hi,
>
>
> This is a request for review of patch to KSL, which partially implements
> bug#4877954 (using relational operators was mentioned there in passing).
>
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4877954
> 4877954: RFE: Special syntax for core interfaces
> Part: Relational operators (<,<=,>,>=) for classes that implement
> Comparable<T>
Disclaimer
i will not comment if this RFE is usefull or not
and i'm not a SUN employee.
>
> General rule:
> if lhs is expression if type that implements Comparable<T> and rhs is
> expression of type that extends/implements T, then relational expression
>
>     lhs op rhs
>
> is considered identical to
>
>     lhs.compareTo((Object) rhs) op 0
>
> where op is one of relational operators (<,<=,>,>=)
I don't see why you want bypass a possible bridge,
i.e. lhs.compareTo((Object) rhs) instead of lhs.compareTo(erasure(rhs)).

Does your implementation cope enums ?
>
>
> Special cases:
>
> 1. Compile time error occurs if lhs or rhs is null (despite null can be
> casted to Comparable<T> or to any T).
>
> 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.
>
> 3. If type of lhs and rhs is numeric type 
i prefer a type that is a subtype of a numeric type,
don't forget type variables and wildcards.
> (i.e. any of byte, Byte, short,
> Short, char, Character, int, Integer, long, Long, float, Float, double,
> Double), then comparison is performed with unboxing (when necessary) and
> immediate primitive value comparison rather than boxing and invoking
> compareTo method. This is necessary to be able compare numbers of any
> primitive or wrapper type with numbers of any other primitive or
> wrapper type. Another reason is to preserve backward source compatibility
> since comparison of primitive floating-point numbers is slightly 
> different
> than comparison of wrappers using compareTo (in cases of -0.0 and NaN).
>
> This change is controlled by command line option -XDcomparable.
>
> 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.
-  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).

minor comment, why in Attr you change tree.lhs.pos() to tree.pos() ?

Rémi