groovy
  1. groovy
  2. GROOVY-2503 MOP 2.0 design inflluencing issues
  3. GROOVY-3364

== operator does not work anymore if Comparable is implemented!

    Details

    • Type: Sub-task Sub-task
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.5.7, 1.6-rc-3
    • Fix Version/s: None
    • Component/s: groovy-runtime
    • Labels:
      None
    • Testcase included:
      yes
    • Patch Submitted:
      Yes
    • Number of attachments :
      0

      Description

      Once a class implements Comparable, the == operator does not work anymore!

      class X implements Comparable {
        int compareTo(Object object) {
          return 0
        }
        boolean equals(Object object) {
          return true;
        }
      }
      
      public void testEqualityOperator() {
        def x = new X()
        def y = new Y()
        assert x == x   // OK 
        assert x == y // OK 
        assert x == 1 // FAIL - Neither compareTo() nor equals() is called!
      }
      

      The location of the bug is in

      DefaultTypeTransform.compareToWithEqualityCheck(Object left, Object right, boolean equalityCheckOnly):
      
              if (equalityCheckOnly) {
                  return -1; // anything other than 0
              }
      

      The fix is:

              if (equalityCheckOnly) {
                return ((Boolean) InvokerHelper.invokeMethod(left, "equals", right)).booleanValue() ? 0 : -1;
              }
      

      An even better fix would be to always use equals for the == operator - but this is already discussed in other issues.

      I've set this to a BLOCKER beause this issue leads to the fact that I cannot use Groovy for relaxed typing - and that makes it useless for many DSLs!

        Activity

        Hide
        Paul King added a comment - - edited

        Just to make sure we are on the same page, I'll also have a go at clarifying things (but I'll slightly over-simplify things in a few spots - hopefully not too much). In Groovy, this code "a.is(b)" is translated into bytecode similar to what Java would produce for "a == b". And this code in Groovy "a == b" is translated into bytecode similar to what Java would produce for "a == b" when dealing with primitives and for "a.equals(b)" when dealing with Objects. So Groovy and Java have different syntax in this regard. You need to have a Groovy (not Java) mindset when interpreting the semantics for those operators. The contract for equals and pointer comparison as defined by Java doesn't change, just the syntax used to represent them. Groovy makes the more common goal (business equality) have the more compact representation (syntax). It also unifies the treatment of primitives and Objects for that symbol (i.e. always business equality). Groovy also tries to add a human number/string/collection equality layer on top (which happens to use compareTo) so that e.g. numbers that humans would consider equal (0, 0L, 0.0f, etc.) are also deemed equal but in some sense this is a red herring to understanding "is" and "==". This issue is about an unintended consequence of how Groovy does the number-aware equality comparisons using compareTo. You might also want to look into the canEqual support within @EqualsAndHashCode.

        Show
        Paul King added a comment - - edited Just to make sure we are on the same page, I'll also have a go at clarifying things (but I'll slightly over-simplify things in a few spots - hopefully not too much). In Groovy, this code " a.is(b) " is translated into bytecode similar to what Java would produce for " a == b ". And this code in Groovy " a == b " is translated into bytecode similar to what Java would produce for " a == b " when dealing with primitives and for " a.equals(b) " when dealing with Objects. So Groovy and Java have different syntax in this regard. You need to have a Groovy (not Java) mindset when interpreting the semantics for those operators. The contract for equals and pointer comparison as defined by Java doesn't change, just the syntax used to represent them. Groovy makes the more common goal (business equality) have the more compact representation (syntax). It also unifies the treatment of primitives and Objects for that symbol (i.e. always business equality). Groovy also tries to add a human number/string/collection equality layer on top (which happens to use compareTo) so that e.g. numbers that humans would consider equal (0, 0L, 0.0f, etc.) are also deemed equal but in some sense this is a red herring to understanding "is" and "==". This issue is about an unintended consequence of how Groovy does the number-aware equality comparisons using compareTo. You might also want to look into the canEqual support within @EqualsAndHashCode .
        Hide
        blackdrag blackdrag added a comment -

        "in Java, X == Y is true, either when X and Y and native types and their values are the same or when X and Y are two references that are pointing to the same instance."
        This is not entirely true, it is not the native types, it is the primitive types. so int==long can be done, but Integer==Long will always yield false. For this primitive type mechanism Java has Widening Primitive Conversion (http://docs.oracle.com/javase/specs/jls/se7/html/jls-5.html#jls-5.1.2)

        Groovy does not follow these exact semantics. Traditionally the number 1 is in Groovy an Integer and not an int. Thus we traditionally do not have primitives at all, only native types, which would be for the integral types int, long, short, byte and BigInteger. Between those we allow conversions in terms of widening and narrowing (byte = int is allowed without cast in Groovy). So unlike what that page may suggest to you, we can neither simply call equals, not can we simply call compareTo, or something like 5==5l would be always false. Thus we have some conversion code depending on the involved types we decide if we call equals or compareTo with or without conversions. In case of 5=5l we will traditionally call https://github.com/groovy/groovy-core/blob/master/src/main/org/codehaus/groovy/runtime/typehandling/LongMath.java#L49 thus it will promote the Integer to a long and then not call compareTo or equals at all.

        Since Groovy 1.8 groovy supports also primitives that stay primitives (before they have been boxed on the first possible chance), so in the case of 5=5l we will do the widening in bytecode and use bytecode instructions to compare the primitive longs instead of going through NumberMath. But if you have a==b and a is an Integer 5 and b a Long 5l, then we still go through that path.

        So unlike Java we have widening not only for primitive types, when we do ==. We have several conversions/actions special for GString/String, our number types, lists and maps. Only if none of these apply we call compareTo for Comparables and equals as last resort. Referential identity not used for == by us at all, unless an equals method we call does that.

        I would say that makes the Groovy logic for == similar in many aspects to Java, but also very different.

        Is for <=>, this is also not purely calling compareTo. Both == and <=> go through https://github.com/groovy/groovy-core/blob/master/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java#L533 the only difference being that equalityCheckOnly is true for == and false for <=>. So we have 5<=>5l return 0 for example. Part of the problem is that compareTo is not required to be defined for any value, like equals. So compareTo is allowed to throw exceptions. Thus, if we have a<=>b, and b is not an instance of the class of a, then we don't call compareTo either and instead produce a GroovyRuntimeException.

        What http://docs.codehaus.org/display/GROOVY/Operator+Overloading is a simplification, maybe an oversimplification in some places.

        Defining the problem for == is not easy, but let me try. Basically we want to make == sense for all primitives, for maps, lists, String, GString, Character and all normal Numbers, which are Short, Byte, Integer, Long, BigInteger, Float, Double and BigDecimal and to some extend the mix of these types. And that in the sense of a somehow equal value. Neither equals, nor compareTo can do that.

        That compareTo is called for an type not included in the set above is surely a point we can discuss. But we cannot ignore the cases above.

        Show
        blackdrag blackdrag added a comment - "in Java, X == Y is true, either when X and Y and native types and their values are the same or when X and Y are two references that are pointing to the same instance." This is not entirely true, it is not the native types, it is the primitive types. so int==long can be done, but Integer==Long will always yield false. For this primitive type mechanism Java has Widening Primitive Conversion ( http://docs.oracle.com/javase/specs/jls/se7/html/jls-5.html#jls-5.1.2 ) Groovy does not follow these exact semantics. Traditionally the number 1 is in Groovy an Integer and not an int. Thus we traditionally do not have primitives at all, only native types, which would be for the integral types int, long, short, byte and BigInteger. Between those we allow conversions in terms of widening and narrowing (byte = int is allowed without cast in Groovy). So unlike what that page may suggest to you, we can neither simply call equals, not can we simply call compareTo, or something like 5==5l would be always false. Thus we have some conversion code depending on the involved types we decide if we call equals or compareTo with or without conversions. In case of 5=5l we will traditionally call https://github.com/groovy/groovy-core/blob/master/src/main/org/codehaus/groovy/runtime/typehandling/LongMath.java#L49 thus it will promote the Integer to a long and then not call compareTo or equals at all. Since Groovy 1.8 groovy supports also primitives that stay primitives (before they have been boxed on the first possible chance), so in the case of 5=5l we will do the widening in bytecode and use bytecode instructions to compare the primitive longs instead of going through NumberMath. But if you have a==b and a is an Integer 5 and b a Long 5l, then we still go through that path. So unlike Java we have widening not only for primitive types, when we do ==. We have several conversions/actions special for GString/String, our number types, lists and maps. Only if none of these apply we call compareTo for Comparables and equals as last resort. Referential identity not used for == by us at all, unless an equals method we call does that. I would say that makes the Groovy logic for == similar in many aspects to Java, but also very different. Is for <=>, this is also not purely calling compareTo. Both == and <=> go through https://github.com/groovy/groovy-core/blob/master/src/main/org/codehaus/groovy/runtime/typehandling/DefaultTypeTransformation.java#L533 the only difference being that equalityCheckOnly is true for == and false for <=>. So we have 5<=>5l return 0 for example. Part of the problem is that compareTo is not required to be defined for any value, like equals. So compareTo is allowed to throw exceptions. Thus, if we have a<=>b, and b is not an instance of the class of a, then we don't call compareTo either and instead produce a GroovyRuntimeException. What http://docs.codehaus.org/display/GROOVY/Operator+Overloading is a simplification, maybe an oversimplification in some places. Defining the problem for == is not easy, but let me try. Basically we want to make == sense for all primitives, for maps, lists, String, GString, Character and all normal Numbers, which are Short, Byte, Integer, Long, BigInteger, Float, Double and BigDecimal and to some extend the mix of these types. And that in the sense of a somehow equal value. Neither equals, nor compareTo can do that. That compareTo is called for an type not included in the set above is surely a point we can discuss. But we cannot ignore the cases above.
        Hide
        Bertrand Francizod added a comment -

        Thanks a lot to both of you for your answers. They are giving me a new light on the problem and I will add elements to this thread as soon as I have digested all of this.

        Show
        Bertrand Francizod added a comment - Thanks a lot to both of you for your answers. They are giving me a new light on the problem and I will add elements to this thread as soon as I have digested all of this.
        Hide
        Patrick Ryan added a comment -

        You have made a cogent argument around the complexities involved - but the bottom line in all of this is that the following does not work in a DSL:

        asset x == 1

        I have a DSL like:

        if( responseTo('MyHeight') == 5 ) .....

        where responseTo returns a complex object which includes the users selected units from the authoring environment, along with other pieces of information.

        For my response object, I was expecting/hoping that either .equals() or .compareTo() would handle this situation.

        Since I cannot change the DSL, because I am trying to create a script to execute the existing DSL, I am in the position of trying to figure out how to support this kind of syntax.

        Given this is my current situation - does anyone have a suggestion on how I could support all of the logical operators for the above DSL snippet?

        Thank you

        Show
        Patrick Ryan added a comment - You have made a cogent argument around the complexities involved - but the bottom line in all of this is that the following does not work in a DSL: asset x == 1 I have a DSL like: if( responseTo('MyHeight') == 5 ) ..... where responseTo returns a complex object which includes the users selected units from the authoring environment, along with other pieces of information. For my response object, I was expecting/hoping that either .equals() or .compareTo() would handle this situation. Since I cannot change the DSL, because I am trying to create a script to execute the existing DSL, I am in the position of trying to figure out how to support this kind of syntax. Given this is my current situation - does anyone have a suggestion on how I could support all of the logical operators for the above DSL snippet? Thank you
        Hide
        blackdrag blackdrag added a comment -

        You know, you can always use an AST transformation and change the AST to use different methods.

        Show
        blackdrag blackdrag added a comment - You know, you can always use an AST transformation and change the AST to use different methods.

          People

          • Assignee:
            Unassigned
            Reporter:
            Peter Rietzler
          • Votes:
            15 Vote for this issue
            Watchers:
            14 Start watching this issue

            Dates

            • Created:
              Updated: