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

Disable overriding via MOP for certain interfaces, classes and methods

    Details

    • Type: Sub-task Sub-task
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.5.6
    • Fix Version/s: None
    • Component/s: groovy-jdk
    • Labels:
      None
    • Number of attachments :
      0

      Description

      Disable overriding via MOP for the following interfaces, classes and methods

      java.lang.CharSequence
       charAt(int)
       length()
       subSequence(int, int)
      
      java.lang.Comparable
       compareTo(T)
      
      java.lang.Number
       byteValue()
       doubleValue()
       floatValue()
       intValue()
       longValue()
       shortValue()
      
      java.lang.Object
       equals(Object)
       getClass()
       hashCode()
       toString()
      
      java.lang.Boolean
       <all methods>
      
      java.lang.Byte
       <all methods>
      
      java.lang.Short
       <all methods>
      
      java.lang.Integer
       <all methods>
      
      java.lang.Long
       <all methods>
      
      java.lang.Double
       <all methods>
      
      java.lang.Float
       <all methods>
      
      java.lang.Enum
       getDeclaringClass()
       name()
       ordinal()
      
      java.lang.String
       <all methods>
      
      java.lang.StringBuffer
       <all methods>
      
      java.lang.StringBuilder
       <all methods>
      

      Overriding these methods

      • does not make sense,
      • adds significant runtime overhead to GDK methods,
      • is not thoroughly respected by runtime libraries, epecially the JRE, and
      • would require to change class file generation (e.g. for concatenated literals).

      This issue is related to http://jira.codehaus.org/browse/GROOVY-2599.

        Activity

        Hide
        blackdrag blackdrag added a comment -

        let us discuss this a bit please... First... there is always a static and dynamic aspect of a class. We can change the dynamic, but not the static aspect. Javacode will normally only act on the static aspects. This is true especially for classes from java.lang, but is a general "problem". For example if you create a class in Groovy or Java and then call a method that exists in the static aspect from Java, then the method in the static aspect will be called, totally ignoring the MetaClass and with it any dynamic aspect. The only way to change that right now would be by changing the bytecode itself, which is possible sometimes, but is not a way we want to go. Your last point is invalid, because the current way is a÷ready a way that respects dynamic methods. For example for "a"+"b" we could have theoretically an alternative plus implementation, which may lead to a different result than "ab".

        When you say "don't overwrite" I guess you actually mean the GDK methods too. But why are interfaces in the list? Any method bound to that interface would normally be overwritten by the actual implementation of the interface anyway.

        As for your point "performance"... it is kind of funny, because we already decided to go away from the pure the static way in GDK methods we try to follow most of the time. For example equals... even if equals(Object) is locked for changes, you can still overload the method by using a more specific parameter type, thus requiring dynamic dispatch because of multi methods. It was a requirement to enable overloaded equals methods in GDK methods. But if we go with equals, then why not with the others too? You see we are trying to improve Groovy's performance big times, but the ultimate goal is still to have most of Groovy written in Groovy itself, especially the GDK methods. We don't fear the performance, because as with 1.6 we have ways to improve and we continue working on that part.

        So if your only reason for "doesn't make sense" is performance then the argument doesn't make sense to me. Your point 3 is a general mismatch between Java and Groovy and is owed to the fact that we cannot change methods as we like for Java. Which is more or less your point 4.

        So I guess you need to explain "doesn't make sense" a bit more.

        Show
        blackdrag blackdrag added a comment - let us discuss this a bit please... First... there is always a static and dynamic aspect of a class. We can change the dynamic, but not the static aspect. Javacode will normally only act on the static aspects. This is true especially for classes from java.lang, but is a general "problem". For example if you create a class in Groovy or Java and then call a method that exists in the static aspect from Java, then the method in the static aspect will be called, totally ignoring the MetaClass and with it any dynamic aspect. The only way to change that right now would be by changing the bytecode itself, which is possible sometimes, but is not a way we want to go. Your last point is invalid, because the current way is a÷ready a way that respects dynamic methods. For example for "a"+"b" we could have theoretically an alternative plus implementation, which may lead to a different result than "ab". When you say "don't overwrite" I guess you actually mean the GDK methods too. But why are interfaces in the list? Any method bound to that interface would normally be overwritten by the actual implementation of the interface anyway. As for your point "performance"... it is kind of funny, because we already decided to go away from the pure the static way in GDK methods we try to follow most of the time. For example equals... even if equals(Object) is locked for changes, you can still overload the method by using a more specific parameter type, thus requiring dynamic dispatch because of multi methods. It was a requirement to enable overloaded equals methods in GDK methods. But if we go with equals, then why not with the others too? You see we are trying to improve Groovy's performance big times, but the ultimate goal is still to have most of Groovy written in Groovy itself, especially the GDK methods. We don't fear the performance, because as with 1.6 we have ways to improve and we continue working on that part. So if your only reason for "doesn't make sense" is performance then the argument doesn't make sense to me. Your point 3 is a general mismatch between Java and Groovy and is owed to the fact that we cannot change methods as we like for Java. Which is more or less your point 4. So I guess you need to explain "doesn't make sense" a bit more.
        Hide
        Alexander Veit added a comment -

        First of all this issue was not intended to deal with GDK methods. "Disable overriding" should be read to refer to overriding the methods defined by the interfaces and classes in the Java runtime library via MOP, e.g.

        Boolean.metaClass.booleanValue = {-> false}
        String.metaClass.toString = {-> "foo"}
        Integer.metaClass.intValue = {-> 42}
        

        These are attempts to change the object's data and/or semantics. Doing so can make a program kind of non-deterministic since it's output may depend on the paths the respective objects follow during program execution. This was my main reason for saying that overriding these methods does not make sense.

        The equals(Object), hashCode(), and compareTo(Object) methods are essential for the concept of object identity in Java. They are widely used e.g. in collections or for sorting. The behaviour of objects that have these methods dynamically overridden is not really foreseeable for the user. Furthermore it is not even clear how the user must override these methods to affect Groovy's concept of equality.

        To clarify point 4: if you have code like

        String.metaClass.toString = {-> "foo"}
         
        //...
         
        def s = "a" + "b"
        

        you would probably have s == "afoo" at runtime with the current byte code. It's quite unclear to me which of the alternatives
        s == "ab"
        s == "foofoo"
        would be desireable instead.

        Show
        Alexander Veit added a comment - First of all this issue was not intended to deal with GDK methods. "Disable overriding" should be read to refer to overriding the methods defined by the interfaces and classes in the Java runtime library via MOP, e.g. Boolean .metaClass.booleanValue = {-> false } String .metaClass.toString = {-> "foo" } Integer .metaClass.intValue = {-> 42} These are attempts to change the object's data and/or semantics. Doing so can make a program kind of non-deterministic since it's output may depend on the paths the respective objects follow during program execution. This was my main reason for saying that overriding these methods does not make sense. The equals(Object), hashCode(), and compareTo(Object) methods are essential for the concept of object identity in Java. They are widely used e.g. in collections or for sorting. The behaviour of objects that have these methods dynamically overridden is not really foreseeable for the user. Furthermore it is not even clear how the user must override these methods to affect Groovy's concept of equality. To clarify point 4: if you have code like String .metaClass.toString = {-> "foo" } //... def s = "a" + "b" you would probably have s == "afoo" at runtime with the current byte code. It's quite unclear to me which of the alternatives s == "ab" s == "foofoo" would be desireable instead.
        Hide
        blackdrag blackdrag added a comment -

        What method a class has to be depending on the path a program takes is what you get when you allow the user to dynamically replace methods. Just imagine I had a library that would once you enter its domain replace all methods with its own methods and restore what was before once you leave that domain.

        I think there are two ways here... one is to simply disallow overriding the methods you mentioned... but then you have the same problem with other methods too. Maybe they are not as critical, but it would mean you try to hide something that will expose itself later anyway. the other way would be to let the user life with this "mismatch" and say that it is his responsibility. Till now I always said, that if you can not solve a problem, then it is better not to hide it, but to let the user see the dangers right away so he can decide himself if he wants that or not. so of course my line is the second way.

        Theoretically there are ways to let Java code respect a dynamically added equals method if the class is written in Groovy. Same for all methods. But we would not be able to override methods on java classes... so if I would follow your argument, then I should lock all classes that are written in Java... at last for replacement and overloading. Or not?

        Show
        blackdrag blackdrag added a comment - What method a class has to be depending on the path a program takes is what you get when you allow the user to dynamically replace methods. Just imagine I had a library that would once you enter its domain replace all methods with its own methods and restore what was before once you leave that domain. I think there are two ways here... one is to simply disallow overriding the methods you mentioned... but then you have the same problem with other methods too. Maybe they are not as critical, but it would mean you try to hide something that will expose itself later anyway. the other way would be to let the user life with this "mismatch" and say that it is his responsibility. Till now I always said, that if you can not solve a problem, then it is better not to hide it, but to let the user see the dangers right away so he can decide himself if he wants that or not. so of course my line is the second way. Theoretically there are ways to let Java code respect a dynamically added equals method if the class is written in Groovy. Same for all methods. But we would not be able to override methods on java classes... so if I would follow your argument, then I should lock all classes that are written in Java... at last for replacement and overloading. Or not?
        Hide
        Alexander Veit added a comment -

        I admit that it is a very serious question which methods should be allowed to be dynamically overridden and which not. A natural approach would be to forbid dynamic overriding of final methods or methods of final classes. But I did not dare to suggest that

        There were actually two reasons that led me to file this issue. First to prevent users from doing unwise things by letting code fail fast (through an exception) instead of letting the code show erratic behaviour at runtime. Second to allow future optimizations for compiler and runtime by letting things not get too complicated (e.g. to avoid calls to dynamically overridden xxxValue() methods in unboxing). Therefore the restriction to the most basic types and semantics.

        Dynamically added methods shouldn't be a problem neither for Groovy objects nor for POJOs, since normally there's no code in the Java domain that would call them.

        Show
        Alexander Veit added a comment - I admit that it is a very serious question which methods should be allowed to be dynamically overridden and which not. A natural approach would be to forbid dynamic overriding of final methods or methods of final classes. But I did not dare to suggest that There were actually two reasons that led me to file this issue. First to prevent users from doing unwise things by letting code fail fast (through an exception) instead of letting the code show erratic behaviour at runtime. Second to allow future optimizations for compiler and runtime by letting things not get too complicated (e.g. to avoid calls to dynamically overridden xxxValue() methods in unboxing). Therefore the restriction to the most basic types and semantics. Dynamically added methods shouldn't be a problem neither for Groovy objects nor for POJOs, since normally there's no code in the Java domain that would call them.
        Hide
        blackdrag blackdrag added a comment -

        Many of the classes you mentioned are final, but not all. Especially equals is neither final nor is Object final.

        Let us assume you want to call equals, then we could theoretically optimize the call into a direct method call under certain circumstances. But are these? First there is the ability to intercept a method call by objects implementing GroovyInterceptable. As I see it, the whole concept of that breaks down if Groovy code is suddenly no longer interceptable. Then the user can use a custom MetaClass to intercept the call... of course both cases are only respected if the call comes from Groovy. Currently we have an easy and understandable explanation for the mismatch: if the call is done from Java, then this additional dynamic logic is ignored. Now if we follow your idea, then how do we explain it? WE disallow it, because we can not guarantee that if the call is done from Java that it will behave as from Groovy? then again, this happens for all methods defined at compile time.

        In short you have not answered my question in the post before... Why not disallow overriding and overloading of methods using the meta class for methods defined at compile time?

        For me this looks very strict and for code that never leaves the Groovy domain it does not even make much sense. Of course we could do great optimizations with this in some cases... but do we want that? It goes against multimethods and more important it goes against dynamic typing.

        Let us say we have this class:

        class X {
          public boolean equals(Object o) { return false;}
          public boolean equals(X x) { return true;}
        }
        

        that sure looks strange from a Java point of view, but look at this code:

          X x = new X();
          Object  o = x;
          assert x.equals(x) != x.equals(o)
        

        in Java we know that assert will pass. We know Java chooses a method signature at compile time and uses the static types to do so. But the big difference here is, that If I have code that is based on instances of X and instances of subclasses of X that I will probably always use the equals(X) method instead of the equals(Object) method. Using equals(X) is fine in your own code, but as soon as you put that into lists or other structures that are based on Object and equals(Object) instead of equals(X) you are screwed. And now answer me why Java does still allow the user to overload equals?

        Sure, the issue here is more serious than my example with Java, but I still do not see disallowing something as the universal medicine. You have to ask how bad things can get and what is the most easy way for the user to prevent the problem or what is the most understandable explanation for the user to understand the problem so he can work around it..

        Show
        blackdrag blackdrag added a comment - Many of the classes you mentioned are final, but not all. Especially equals is neither final nor is Object final. Let us assume you want to call equals, then we could theoretically optimize the call into a direct method call under certain circumstances. But are these? First there is the ability to intercept a method call by objects implementing GroovyInterceptable. As I see it, the whole concept of that breaks down if Groovy code is suddenly no longer interceptable. Then the user can use a custom MetaClass to intercept the call... of course both cases are only respected if the call comes from Groovy. Currently we have an easy and understandable explanation for the mismatch: if the call is done from Java, then this additional dynamic logic is ignored. Now if we follow your idea, then how do we explain it? WE disallow it, because we can not guarantee that if the call is done from Java that it will behave as from Groovy? then again, this happens for all methods defined at compile time. In short you have not answered my question in the post before... Why not disallow overriding and overloading of methods using the meta class for methods defined at compile time? For me this looks very strict and for code that never leaves the Groovy domain it does not even make much sense. Of course we could do great optimizations with this in some cases... but do we want that? It goes against multimethods and more important it goes against dynamic typing. Let us say we have this class: class X { public boolean equals( Object o) { return false ;} public boolean equals(X x) { return true ;} } that sure looks strange from a Java point of view, but look at this code: X x = new X(); Object o = x; assert x.equals(x) != x.equals(o) in Java we know that assert will pass. We know Java chooses a method signature at compile time and uses the static types to do so. But the big difference here is, that If I have code that is based on instances of X and instances of subclasses of X that I will probably always use the equals(X) method instead of the equals(Object) method. Using equals(X) is fine in your own code, but as soon as you put that into lists or other structures that are based on Object and equals(Object) instead of equals(X) you are screwed. And now answer me why Java does still allow the user to overload equals? Sure, the issue here is more serious than my example with Java, but I still do not see disallowing something as the universal medicine. You have to ask how bad things can get and what is the most easy way for the user to prevent the problem or what is the most understandable explanation for the user to understand the problem so he can work around it..
        Hide
        Alexander Veit added a comment -

        Why not disallow overriding and overloading of methods using the meta class for methods defined at compile time?

        Can anyone foresee which real world use case can benefit from dynamic overriding of POJO methods? Probably not. So I really do not want to speak for being that restrictive.

        [...] And now answer me why Java does still allow the user to overload equals?

        OK, Java allows overloading equals. Groovy allows dynamic overriding of equals. Both probably makes little sense, but the user has to know what she or he is doing. The explanation that dynamic aspects don't work in the Java domain may suffice. It is a valid point of view.

        But what about the byteValue(), doubleValue(), floatValue(), intValue(), longValue() and shortValue() of Number and the toString() methods of String, StringBuilder and StringBuffer?

        Let's suppose we want to dynamically override the Integer semantics to cover non-negative integers only. The arithmetic should be as usual, except the minus operation which should be defined as x - y := ||x| - |y||:

        
        Integer.metaClass.intValue = {->
        	// access private member to avoid stack overflow [1]
        	delegate.value < 0 ? -delegate.value : delegate.value
        }
        
        Integer.metaClass.toString = {->
        	Integer.toString(delegate.intValue())
        }
        
        Integer.metaClass.equals = {Object other ->
        	assert false : "never called by this script"
        }
        
        Integer.metaClass.compareTo = {Object other ->
        	assert false : "never called by this script"
        }
        
        assert 0.intValue() == 0
        assert 42.intValue() == 42
        assert (-42).intValue() == 42
        
        assert 0.toString() == "0"
        assert 42.toString() == "42"
        assert (-42).toString() == "42"
        
        // [2]
        assert -1 != 1
        assert new Integer(-1) != new Integer(1)
        
        Integer i
        
        // [3]
        i = 3 + -4
        assert i == -1 // expected to be == 7 [3a]
        assert i.toString() == "1"
        
        i = -4 + 3
        assert i == -1 // expected to be == 7 [3b]
        assert i.toString() == "1"
        
        // [4]
        assert Math.max(1, -2) == 1 // could it be 2?
        
        sw = new StringWriter()
        out = new PrintWriter(sw, true)
        print(-42)
        assert sw.toString() == "-42" // expected to be "42" [5]
        
        

        The above script runs without assertion errors. This raises some questions.

        1) How would Integer#intValue() be dynamically overridden in future Groovy versions that are supposed to respect the private modifier on fields and methods?

        2) Should DefaultTypeTransformation#compareEqual respect the new intValue()? [2] is pure Groovy, so it probably should.

        3) Should any GDK method respect the new intValue()? Again, [3] is pure Groovy, so it probably should. Note that [3a] together with [3b] requires that both the lhs and rhs operands go through the MOP to get the value.

        4) Math.max(1, -2) == 2? Here the Java domain ignores the dynamic aspect, so it's probably ok that we have 1 as the result. If the new intValue() should be respected here, then probably all unboxing methods had to go through the MOP.

        5) Should GroovyObjects that are part of the Groovy API or runtime respect the new intValue()? [5] is pure Groovy, so they probably should.

        6) What about return values and boxing? Probably there are no changes required.

        I think support - even at a low level - for dynamic overriding of these methods would become rather expensive (even more if String

        {Builder,Buffer}

        .toString() is overridden). Maybe too expensive compared to it's benefit, since overriding is a really rare case for this methods and there are better ways to solve problems as the one above.

        Show
        Alexander Veit added a comment - Why not disallow overriding and overloading of methods using the meta class for methods defined at compile time? Can anyone foresee which real world use case can benefit from dynamic overriding of POJO methods? Probably not. So I really do not want to speak for being that restrictive. [...] And now answer me why Java does still allow the user to overload equals? OK, Java allows overloading equals. Groovy allows dynamic overriding of equals. Both probably makes little sense, but the user has to know what she or he is doing. The explanation that dynamic aspects don't work in the Java domain may suffice. It is a valid point of view. But what about the byteValue(), doubleValue(), floatValue(), intValue(), longValue() and shortValue() of Number and the toString() methods of String, StringBuilder and StringBuffer? Let's suppose we want to dynamically override the Integer semantics to cover non-negative integers only. The arithmetic should be as usual, except the minus operation which should be defined as x - y := ||x| - |y||: Integer .metaClass.intValue = {-> // access private member to avoid stack overflow [1] delegate.value < 0 ? -delegate.value : delegate.value } Integer .metaClass.toString = {-> Integer .toString(delegate.intValue()) } Integer .metaClass.equals = { Object other -> assert false : "never called by this script" } Integer .metaClass.compareTo = { Object other -> assert false : "never called by this script" } assert 0.intValue() == 0 assert 42.intValue() == 42 assert (-42).intValue() == 42 assert 0.toString() == "0" assert 42.toString() == "42" assert (-42).toString() == "42" // [2] assert -1 != 1 assert new Integer (-1) != new Integer (1) Integer i // [3] i = 3 + -4 assert i == -1 // expected to be == 7 [3a] assert i.toString() == "1" i = -4 + 3 assert i == -1 // expected to be == 7 [3b] assert i.toString() == "1" // [4] assert Math .max(1, -2) == 1 // could it be 2? sw = new StringWriter() out = new PrintWriter(sw, true ) print(-42) assert sw.toString() == "-42" // expected to be "42" [5] The above script runs without assertion errors. This raises some questions. 1) How would Integer#intValue() be dynamically overridden in future Groovy versions that are supposed to respect the private modifier on fields and methods? 2) Should DefaultTypeTransformation#compareEqual respect the new intValue()? [2] is pure Groovy, so it probably should. 3) Should any GDK method respect the new intValue()? Again, [3] is pure Groovy, so it probably should. Note that [3a] together with [3b] requires that both the lhs and rhs operands go through the MOP to get the value. 4) Math.max(1, -2) == 2? Here the Java domain ignores the dynamic aspect, so it's probably ok that we have 1 as the result. If the new intValue() should be respected here, then probably all unboxing methods had to go through the MOP. 5) Should GroovyObjects that are part of the Groovy API or runtime respect the new intValue()? [5] is pure Groovy, so they probably should. 6) What about return values and boxing? Probably there are no changes required. I think support - even at a low level - for dynamic overriding of these methods would become rather expensive (even more if String {Builder,Buffer} .toString() is overridden). Maybe too expensive compared to it's benefit, since overriding is a really rare case for this methods and there are better ways to solve problems as the one above.
        Hide
        blackdrag blackdrag added a comment -

        The right way to implement a non negative integer in Groovy would be by writing your own number class and probably a asType method where you add a tpye conversion to your type for Integer. I agree it makes not much sense to overwrite intValue in such a manner, but on the hand, if the user wants that... of course you noticed that the Groovy runtime is not interested in the dynamically added methods in these cases.

        I must say, instead of not allowing to override them I would personally prefer the current way.. but that is hiding the problem I guess.

        Show
        blackdrag blackdrag added a comment - The right way to implement a non negative integer in Groovy would be by writing your own number class and probably a asType method where you add a tpye conversion to your type for Integer. I agree it makes not much sense to overwrite intValue in such a manner, but on the hand, if the user wants that... of course you noticed that the Groovy runtime is not interested in the dynamically added methods in these cases. I must say, instead of not allowing to override them I would personally prefer the current way.. but that is hiding the problem I guess.
        blackdrag blackdrag made changes -
        Field Original Value New Value
        Parent GROOVY-2503 [ 61571 ]
        Issue Type Improvement [ 4 ] Sub-task [ 7 ]

          People

          • Assignee:
            Unassigned
            Reporter:
            Alexander Veit
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated: