Uploaded image for project: 'Groovy'
  1. Groovy
  2. GROOVY-3175

Some generated fields not marked as synthetic

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 1.6-beta-1, 1.6-beta-2
    • 1.6-rc-2, 1.7-beta-1
    • bytecode
    • None

    Description

      Some generated fields that should be marked as synthetic are not.

      Given:

      class MyService {
         def thing
         def something() { }
         def anotherSomething() { assert true }
      }
      

      We see:

      MyService.getDeclaredFields().grep {  it.synthetic }
      ==>
      [public static final java.lang.Class MyService.$ownClass, private static java.lang.Class MyService.$class$java$lang$Class, 
      private static java.lang.Class MyService.$class$groovy$lang$MetaClass, 
      private static java.lang.Class MyService.$class$MyService, private static java.lang.ref.SoftReference MyService.$callSiteArray]
      
      MyService.getDeclaredFields().grep {  !it.synthetic }
      ==>
      [private java.lang.Object MyService.thing, private static org.codehaus.groovy.reflection.ClassInfo MyService.$staticClassInfo, 
      private transient groovy.lang.MetaClass MyService.metaClass, public static java.lang.Long MyService.__timeStamp, 
      public static java.lang.Long MyService.__timeStamp__239_neverHappen1227930452630]
      

      The only non-synthetic field should be MyService.thing, the rest are synthetic.

      Attachments

        Activity

          on further investigation I found, that the property field needs to be synthetic or else the compiler won't be able to replace the filed in case the user gives an additional field. If the synthetic flag is not set the compiler is unable to see if the existing field is given by the user or not. As a result the test above will now return no fields that are not synthetic

          blackdrag Jochen Theodorou added a comment - on further investigation I found, that the property field needs to be synthetic or else the compiler won't be able to replace the filed in case the user gives an additional field. If the synthetic flag is not set the compiler is unable to see if the existing field is given by the user or not. As a result the test above will now return no fields that are not synthetic
          jimwhite James P. White added a comment -

          Good. I thought the field should be synthetic (it is a JavaBean property by virtue of the getter/setter which is what the user defines with the def, and so the field is internal implementation) but the groovy-dev discussion said otherwise.

          Problem now though is that the build is broken for retro, even with your workaround attempt. I tried all kinds of things trying to make it work, but I suspect a bug somewhere.

          jimwhite James P. White added a comment - Good. I thought the field should be synthetic (it is a JavaBean property by virtue of the getter/setter which is what the user defines with the def, and so the field is internal implementation) but the groovy-dev discussion said otherwise. Problem now though is that the build is broken for retro, even with your workaround attempt. I tried all kinds of things trying to make it work, but I suspect a bug somewhere.

          well, I think the flag is not set pre java5. So even if I know the value, I cannot ask for it, because the JVM does not fill that flag in for Field.getModifiers(). The only solution seems to be to move the test into a vm5 package.

          blackdrag Jochen Theodorou added a comment - well, I think the flag is not set pre java5. So even if I know the value, I cannot ask for it, because the JVM does not fill that flag in for Field.getModifiers(). The only solution seems to be to move the test into a vm5 package.
          jimwhite James P. White added a comment -

          Synthetic has has been a modifier since JDK 1.1:

          http://java.sun.com/docs/books/jvms/second_edition/html/ClassFile.doc.html#88570

          The Member.isSynthetic method is JDK 1.5, which your change to testing for the bit would resolve.

          The problem is that we get this error for {{...getDeclaredMethods().grep

          { ...}

          }}:

          http://bamboo.ci.codehaus.org/build/viewBuildResultsFailedTests.action?buildKey=GROOVY-CORE14&buildNumber=1800

          I tried all kinds of changes to the code (using findAll, converting to List, not using assertScript...) and always get that "<not implemented yet..." error.

          I did most of adding a vm5 for the test case, but then it would become the only vm5 UberTestCaseBugs. I think there is some sort of compiler bug or something like that rather than an issue with the test case itself.

          jimwhite James P. White added a comment - Synthetic has has been a modifier since JDK 1.1: http://java.sun.com/docs/books/jvms/second_edition/html/ClassFile.doc.html#88570 The Member.isSynthetic method is JDK 1.5, which your change to testing for the bit would resolve. The problem is that we get this error for {{...getDeclaredMethods().grep { ...} }}: http://bamboo.ci.codehaus.org/build/viewBuildResultsFailedTests.action?buildKey=GROOVY-CORE14&buildNumber=1800 I tried all kinds of changes to the code (using findAll , converting to List, not using assertScript ...) and always get that "<not implemented yet..." error. I did most of adding a vm5 for the test case, but then it would become the only vm5 UberTestCaseBugs . I think there is some sort of compiler bug or something like that rather than an issue with the test case itself.

          sure, synthetic exists since early times in java as modifier in the bytecode. But that does not mean that modifier is accessible through the reflection API. You will find that isSythetic() is not part of the API before 1.5, so it is very legal for a VM to not to provide the value of that flag. The VM doesn't need this value anyway.

          That "<not implemented yet..." is not the error, it is part of the error message, that would have to write out how the closure looks like. You can save the value of the left side in a variable and then compare the results, but the assert will still fail, because the synthetic flag is not accessible. As a result the grep/findAll will return too many entries. In other words: with only reflection this can't work on Java4.

          I am very sure there is no compiler bug with this test

          blackdrag Jochen Theodorou added a comment - sure, synthetic exists since early times in java as modifier in the bytecode. But that does not mean that modifier is accessible through the reflection API. You will find that isSythetic() is not part of the API before 1.5, so it is very legal for a VM to not to provide the value of that flag. The VM doesn't need this value anyway. That "<not implemented yet..." is not the error, it is part of the error message, that would have to write out how the closure looks like. You can save the value of the left side in a variable and then compare the results, but the assert will still fail, because the synthetic flag is not accessible. As a result the grep/findAll will return too many entries. In other words: with only reflection this can't work on Java4. I am very sure there is no compiler bug with this test
          jimwhite James P. White added a comment -

          You're right. Reflection on JDK 1.4 does not set the synthetic modifier.

          Test case disabled when JDK < 1.5.

          jimwhite James P. White added a comment - You're right. Reflection on JDK 1.4 does not set the synthetic modifier. Test case disabled when JDK < 1.5.

          People

            blackdrag Jochen Theodorou
            jimwhite James P. White
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: