groovy
  1. groovy
  2. GROOVY-4457

generic type declarations leaking across all files in a build

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.7.5
    • Fix Version/s: 1.8.5, 2.0-beta-2, 1.7.11
    • Component/s: Compiler
    • Labels:
      None
    • Number of attachments :
      1

      Description

      Simple file, A.groovy:

      class A<String> {
      }
      
      class B {
        void foo(String s) {}
      }
      

      groovyc A.groovy
      javap -private B | grep foo

      produces:

      public void foo(java.lang.Object);
      

      The 'String' is treated as a type parameter name. The reference 'String' in the foo method is mapped to this type parameter (clearly it shouldn't be) and when producing the code, String is erased to its upper bound of Object, hence foo(Object) in the bytecode.

      Change it to this:

      class A<T> {
      }
      
      class B {
        void foo(String s) {}
      }
      

      and it produces what you expect:

       public void foo(java.lang.String);
      

      The problem is the genericParameterNames collection in the ResolveVisitor is never cleared, only augmented with new entries.

      My solution that seems to work in groovy eclipse is to clear the parameter names at the end of visiting a classnode in ResolveVisitor. So at the end of

      public void visitClass(ClassNode node) {
      

      add

      genericParameterNames.clear();
      

        Issue Links

          Activity

          Hide
          blackdrag blackdrag added a comment - - edited

          since Roshan seems not to have capacity to fix this (which is not his fault), there is no progress atm

          Show
          blackdrag blackdrag added a comment - - edited since Roshan seems not to have capacity to fix this (which is not his fault), there is no progress atm
          Hide
          blackdrag blackdrag added a comment - - edited

          I guess someone else will have to take this over, unless Roshan says he still wants to fix this one

          Show
          blackdrag blackdrag added a comment - - edited I guess someone else will have to take this over, unless Roshan says he still wants to fix this one
          Hide
          Andy Clement added a comment -

          If I can find time, I'll take a proper look at the right fix.

          Show
          Andy Clement added a comment - If I can find time, I'll take a proper look at the right fix.
          Hide
          CÚdric Champeau added a comment -

          Master includes the fix which breaks the non static inner class case. I will try to commit a fix.

          Show
          CÚdric Champeau added a comment - Master includes the fix which breaks the non static inner class case. I will try to commit a fix.
          Hide
          CÚdric Champeau added a comment -

          Here is the patch against master. Added a unit test for the non static inner class case. If you are ok, I will commit it.

          Show
          CÚdric Champeau added a comment - Here is the patch against master. Added a unit test for the non static inner class case. If you are ok, I will commit it.

            People

            • Assignee:
              CÚdric Champeau
              Reporter:
              Andy Clement
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: