Details

    • Type: Sub-task Sub-task
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Compiler
    • Labels:
      None
    • Environment:
      Ubuntu 6.10 Edgy Eft + Groovy r4630
    • Number of attachments :
      0

      Description

      The following code appears to show that final is being applied inconsistently. A final list can be amended but an object that manipulates a lsit that is final cannot. In the former case the final is being applied to the reference and in the later, it is being applied to the object.

       class Blah {
        def list = []
         public plus ( item ) {
           list += [ item ]
           return this
         }
      }
      
      class Foobar {
         final static blah = new Blah ( )
      }
      
      final x = []
      x += [1]
      println ( x )
      
      Foobar.blah += 1
      println ( Foobar.blah.list ) 
      
      > groovy finalProblem.groovy
      [1]
      Caught: java.lang.IllegalAccessException: Field is final
      at finalProblem.run(finalProblem.groovy:17)
      at finalProblem.main(finalProblem.groovy)

        Issue Links

          Activity

          Hide
          blackdrag blackdrag added a comment -

          You get a runtime error, is that not OK for you?
          The problem is, that we could provide a compile time error here, but only because a static field is used. The compiler might not be able to ensure this for fields used on instances.

          Show
          blackdrag blackdrag added a comment - You get a runtime error, is that not OK for you? The problem is, that we could provide a compile time error here, but only because a static field is used. The compiler might not be able to ensure this for fields used on instances.
          Hide
          Russel Winder added a comment -

          The issue is not when the error message occurs, runtime is fine, the problem is that there is an error here at all.

          The problem is that the expression statement:

          Foobar.blah.plus ( 2 )
          

          works fine, but the supposedly same semantic statement:

          Foobar.blah += 1
          

          fails. I guess the issue is whether the += operator is seen as an assignment of the reference (which it isn't semantically even though it is syntactically) or whether it is a modifier of the object as the plus method call is.

          The issue is one of consistency, and I am not sure Groovy has it just now.

          Show
          Russel Winder added a comment - The issue is not when the error message occurs, runtime is fine, the problem is that there is an error here at all. The problem is that the expression statement: Foobar.blah.plus ( 2 ) works fine, but the supposedly same semantic statement: Foobar.blah += 1 fails. I guess the issue is whether the += operator is seen as an assignment of the reference (which it isn't semantically even though it is syntactically) or whether it is a modifier of the object as the plus method call is. The issue is one of consistency, and I am not sure Groovy has it just now.
          Hide
          blackdrag blackdrag added a comment -

          I think I do not understand yet.

          a += 1

          is not the same as

          a.plus(1)

          it is the same as

          a = a.plus(1)

          so if a is final, the assignment will fail, but not the plus. In the case above Foobar.blah.plus ( 2 ) means to create a new list with the additional item 2 and assign the new list to the old one. The list stored in blah, but not final, so no exception. Foobar.blah += 1, is the same with two differences, the "2" is a "1" now and in the end I am assigning a new value to blah. Since blah is a final field, the VM will throw the runtime message. There is nothing wrong with that.

          The only thing Groovy in combination with the VM ignores here is the final modifier to local variables. Yes, for this we could write a compile time test and throw a compile time error.

          Show
          blackdrag blackdrag added a comment - I think I do not understand yet. a += 1 is not the same as a.plus(1) it is the same as a = a.plus(1) so if a is final, the assignment will fail, but not the plus. In the case above Foobar.blah.plus ( 2 ) means to create a new list with the additional item 2 and assign the new list to the old one. The list stored in blah, but not final, so no exception. Foobar.blah += 1, is the same with two differences, the "2" is a "1" now and in the end I am assigning a new value to blah. Since blah is a final field, the VM will throw the runtime message. There is nothing wrong with that. The only thing Groovy in combination with the VM ignores here is the final modifier to local variables. Yes, for this we could write a compile time test and throw a compile time error.
          Hide
          Russel Winder added a comment -

          I have no problem with the above, it is reasonable and consistent (sort of

          However, if this is the case then shouldn't:

          final x = []
          x += [ 1 ]
          

          result in an exception for exactly the same reasons?

          If not then we need to introduce language to describe the difference. Python has done this using the terms mutable and immutable. Tuples and Lists behave differently under +=.

          Show
          Russel Winder added a comment - I have no problem with the above, it is reasonable and consistent (sort of However, if this is the case then shouldn't: final x = [] x += [ 1 ] result in an exception for exactly the same reasons? If not then we need to introduce language to describe the difference. Python has done this using the terms mutable and immutable. Tuples and Lists behave differently under +=.
          Hide
          blackdrag blackdrag added a comment -

          final on methods and fields is tested by the VM, final on local variables is not. So groovy must check that and it doesn't. So yes, this is a bug, inconsistency or what you want to call it.

          Show
          blackdrag blackdrag added a comment - final on methods and fields is tested by the VM, final on local variables is not. So groovy must check that and it doesn't. So yes, this is a bug, inconsistency or what you want to call it.
          Hide
          Tord Alenljung added a comment -

          It seems like final on fields is not checked according to the following example:

          class C {
              final a = 0
              void changeA() {
                  a = a + 1
              }
              final b = 0
              final setB(val) {
                  b = val
              }
          }
          c = new C()
          c.changeA()
          c.changeA()
          assert c.a == 2
          c.b = 1
          assert c.b == 1
          
          Show
          Tord Alenljung added a comment - It seems like final on fields is not checked according to the following example: class C { final a = 0 void changeA() { a = a + 1 } final b = 0 final setB(val) { b = val } } c = new C() c.changeA() c.changeA() assert c.a == 2 c.b = 1 assert c.b == 1
          Hide
          Paul King added a comment -

          Just commenting on status of current implementation:
          Tord's example is checked - it is on fields and gives an appropriate error.
          Russel's example isn't checked yet - local variable case.

          Show
          Paul King added a comment - Just commenting on status of current implementation: Tord's example is checked - it is on fields and gives an appropriate error. Russel's example isn't checked yet - local variable case.
          Hide
          Paul King added a comment - - edited

          This is listed as a subtask of GROOVY-4681 but in reality, doesn't the description of GROOVY-4681 capture the remaining issue(s)? I.e. can we close one of them as a duplicate?

          Show
          Paul King added a comment - - edited This is listed as a subtask of GROOVY-4681 but in reality, doesn't the description of GROOVY-4681 capture the remaining issue(s)? I.e. can we close one of them as a duplicate?

            People

            • Assignee:
              blackdrag blackdrag
              Reporter:
              Russel Winder
            • Votes:
              3 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated: