groovy
  1. groovy
  2. GROOVY-3089

compile error with covariant bridge method added for already existing method

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.6-beta-2
    • Fix Version/s: 1.6-rc-1, 1.5.8, 1.7-beta-1
    • Component/s: None
    • Labels:
      None
    • Environment:
      Win XP SP2, JRE 1.6.0
    • Number of attachments :
      2

      Description

      Open groovyConsole, enter:

      class Foo {
      @Delegate Date d = new Date();
      }

      Press Ctrl+Enter. Error displayed:

      org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed, Script14: -1: Repetitive method name/signature for method 'int compareTo(java.lang.Object)' in class 'Foo'.
      @ line -1, column -1.Script14: -1: Repetitive method name/signature for method 'int compareTo(java.lang.Object)' in class 'Foo'.
      @ line -1, column -1.
      2 errors

      Note: changing Date to other classes such as Map works fine, seems to be something specific about Date.

        Activity

        Hide
        Roshan Dawrani added a comment -

        Hi,
        Please find attached the fix patches for the issue for versions 1.6-RC-1 and 1.7-beta-1.

        The error was found in co-variant algorithm in Verifier.

        The compilation error reported in the bug was originating from ClassCompletionVerifier->checkRepetitiveMethod() because it was seeing duplicate methods nodes for "int compareTo(Object)" added on ClassNode [Foo].

        1) First method node is added by DelegateASTTransformation.addDelegateMethod() because java.util.Date class has "int compareTo(Object) [synthetic = true]"

        2) Second method node is added by Verifier.addCovariantMethods().

        In case of "java.util.Date implements java.lang.Comparable<Date>", what happens is that bridge method that groovy Verifier tries to add is already added by Java and java.util.Date class has following 2 methods:

        1) public int compareTo(Date) [synthetic = false]
        2) public int compareTo(Object) [synthetic = true] (Since this bridge method is already provided by Java, it does not need to be provided by Groovy and that is the fix done on co-variant algorithm.)

        Co-variant algorithm implemented in org.codehaus.groovy.classgen.Verifier had the following issue:

        • For class Foo {@Delegate Date ....}

          , it comes across interface method "int compareTo(Object)" and method "int compareTo(Date)" on ClassNode[Foo] (added earlier by DelegateASTTransformation.addDelegateMethod()) and it incorrectly concludes that it needs to provide a bridge method "int compareTo(Object)" ignoring the possibility that it may already be present on ClassNode (as in case of java.util.Date).

        To fix this, I have added an additional check in Verifier.storeMissingCovariantMethods() before adding the bridge method to ClassNode. The check is done only after co-variant case is identified by Groovy and a bridgeMethod is prepared by it so that performance hit incurred by this additional check is not added to non-co-variant cases.

        With the patch, the following goes through:

        class Foo {
            @Delegate Date d = new Date();
        }
        
        def foo = new Foo()
        def d1 = new Date(); d1++
        println foo.compareTo(d1)
        

        Hope it helps.
        Roshan

        Show
        Roshan Dawrani added a comment - Hi, Please find attached the fix patches for the issue for versions 1.6-RC-1 and 1.7-beta-1. The error was found in co-variant algorithm in Verifier. The compilation error reported in the bug was originating from ClassCompletionVerifier->checkRepetitiveMethod() because it was seeing duplicate methods nodes for "int compareTo(Object)" added on ClassNode [Foo] . 1) First method node is added by DelegateASTTransformation.addDelegateMethod() because java.util.Date class has "int compareTo(Object) [synthetic = true] " 2) Second method node is added by Verifier.addCovariantMethods(). In case of "java.util.Date implements java.lang.Comparable<Date>", what happens is that bridge method that groovy Verifier tries to add is already added by Java and java.util.Date class has following 2 methods: 1) public int compareTo(Date) [synthetic = false] 2) public int compareTo(Object) [synthetic = true] (Since this bridge method is already provided by Java, it does not need to be provided by Groovy and that is the fix done on co-variant algorithm.) Co-variant algorithm implemented in org.codehaus.groovy.classgen.Verifier had the following issue: For class Foo {@Delegate Date ....} , it comes across interface method "int compareTo(Object)" and method "int compareTo(Date)" on ClassNode [Foo] (added earlier by DelegateASTTransformation.addDelegateMethod()) and it incorrectly concludes that it needs to provide a bridge method "int compareTo(Object)" ignoring the possibility that it may already be present on ClassNode (as in case of java.util.Date). To fix this, I have added an additional check in Verifier.storeMissingCovariantMethods() before adding the bridge method to ClassNode. The check is done only after co-variant case is identified by Groovy and a bridgeMethod is prepared by it so that performance hit incurred by this additional check is not added to non-co-variant cases. With the patch, the following goes through: class Foo { @Delegate Date d = new Date(); } def foo = new Foo() def d1 = new Date(); d1++ println foo.compareTo(d1) Hope it helps. Roshan
        Hide
        Roshan Dawrani added a comment -

        Actually, since the root cause is found in co-variance algorithm and not really in @Delegate feature, the issue is also present in version 1.5.8.

        Below is the test case to effect the same "Repetitive method name/signature for method" error in 1.5.8 (since it does not have @Delegate feature). I am attaching a patch that has the fix for 1.5.8 and also the additional test case to highlight the co-variance error.

        class Groovy2089CovarianceTest extends GroovyTestCase {
            def void testCovarianceMethodAddWithGenericsInterfaces() {
            	new GroovyShell().parse(
                	"""
        				interface IBar<T>{
        				    int testMethod(T obj)
        				}
        				
        				class Bar implements IBar<Date>{
        				    int testMethod(Date dt){}
        				    int testMethod(Object obj){}
        				}
                        1
                	"""
            		)
            }
        }
        

        rgds,
        Roshan

        Show
        Roshan Dawrani added a comment - Actually, since the root cause is found in co-variance algorithm and not really in @Delegate feature, the issue is also present in version 1.5.8. Below is the test case to effect the same "Repetitive method name/signature for method" error in 1.5.8 (since it does not have @Delegate feature). I am attaching a patch that has the fix for 1.5.8 and also the additional test case to highlight the co-variance error. class Groovy2089CovarianceTest extends GroovyTestCase { def void testCovarianceMethodAddWithGenericsInterfaces() { new GroovyShell().parse( """ interface IBar<T>{ int testMethod(T obj) } class Bar implements IBar<Date>{ int testMethod(Date dt){} int testMethod( Object obj){} } 1 """ ) } } rgds, Roshan
        Hide
        blackdrag blackdrag added a comment -

        I did not fix exactly like you suggested, instead I did remove the surplus methods before adding them to the ClassNode.

        Show
        blackdrag blackdrag added a comment - I did not fix exactly like you suggested, instead I did remove the surplus methods before adding them to the ClassNode.
        Hide
        Roshan Dawrani added a comment -

        Much better fix in terms of implementation. Thanks.

        rgds,
        Roshan

        Show
        Roshan Dawrani added a comment - Much better fix in terms of implementation. Thanks. rgds, Roshan

          People

          • Assignee:
            blackdrag blackdrag
            Reporter:
            Simon
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: