Final field update in unreachable for loop update section

[Vicente Romero]

Hi Tagir,

What you are saying is very sensible. We were discussing yesterday about 
this same thing internally, as you observed javac and the spec are not 
sync in this area. We will get back to you later, thanks for the discussion,

Vicente

On 02/16/2018 05:26 AM, Tagir Valeev wrote:
> Hello! Thank you for investigating this. On a second look the javac 
> behavior looks somewhat logical.
> The for loop execution looks like this:
>
>
>  /-----------------------------> (next statement)
>                        |
> initialization --> condition --> body -X-> incrementation
>                        ^                      |
>  \-----------------------/
>
> Having a "break" statement we removed an edge marked with X. However 
> there's still an "incrementation->condition->body" chain, even though 
> the incrementation is unreachable. So javac likely goes along this 
> path and finds that "x=2" (body) is possible after "x=1" (increment). 
> To my understanding, usually spec works like this: unreachable 
> statements are still analyzed. The x is not DU after increment, thus 
> it should not be DU before empty condition, thus it should not be DU 
> before body. 16.2.12 says:
>
> Vis definitely unassigned before the condition part of 
> the|for|statement iff all of the following are true:
>
>  *
>
>     Vis definitely unassigned after the initialization part of
>     the|for|statement.
>
>  *
>
>     AssumingVis definitely unassigned before the condition part of
>     the|for|statement,Vis definitely unassigned after the contained
>     statement.
>
>  *
>
>     AssumingVis definitely unassigned before the contained
>     statement,Vis definitely unassigned before
>     every|continue|statement for which the|for|statement is the
>     continue target.
>
>
> This would be perfectly logical were condition part executed either 
> after initialization or after body (or continue statement within the 
> body). However actually it's executed either after initialization or 
> after the incrementation. Probably this part should be reformulated in 
> the following way:
>
> Vis definitely unassigned before the condition part of 
> the|for|statement iff all of the following are true:
>
>  *
>
>     Vis definitely unassigned after the initialization part of
>     the|for|statement.
>
>  *
>
>     AssumingVis definitely unassigned before the condition part of
>     the|for|statement,Vis definitely unassigned after the
>     incrementation part.
>
> This way it would be aligned with javac implementation and sound more 
> logical.
>
> Am I missing something?
>
> Regards,
> Tagir Valeev.
>
> On Fri, Feb 16, 2018 at 3:39 AM, Vicente Romero 
> <vicente.romero at oracle.com <mailto:vicente.romero at oracle.com>> wrote:
>
>     Thanks Dan for evaluating this one, certainly this set of rules is
>     tricky to follow. I have created bug [1] to track this issue,
>
>     Vicente
>
>     [1] https://bugs.openjdk.java.net/browse/JDK-8198245
>     <https://bugs.openjdk.java.net/browse/JDK-8198245>
>
>
>     On 02/15/2018 03:16 PM, Dan Smith wrote:
>>     Thanks for the interesting test case.
>>
>>     You're pushing on this part of the language spec:
>>     https://docs.oracle.com/javase/specs/jls/se9/html/jls-16.html#jls-16.2.12
>>     <https://docs.oracle.com/javase/specs/jls/se9/html/jls-16.html>
>>     https://docs.oracle.com/javase/specs/jls/se9/html/jls-16.html#jls-16.2.13
>>     <https://docs.oracle.com/javase/specs/jls/se9/html/jls-16.html#jls-16.2.13>
>>
>>>     On Feb 7, 2018, at 7:49 PM, Tagir Valeev <amaembo at gmail.com
>>>     <mailto:amaembo at gmail.com>> wrote:
>>>
>>>     public class Sample1 {
>>>      final int x;
>>>
>>>      Sample1() {
>>>        for(;;x=1) {
>>>          x=2;
>>>          break;
>>>        }
>>>      }
>>>     }
>>
>>     An error should only occur if 'x' is assigned to where it is
>>     definitely unassigned ("DU").
>>
>>     The rules say that:
>>
>>     - x is DU before the for loop
>>
>>     - x is DU after the (empty) initialization
>>
>>     - assuming x is DU before the (empty) condition:
>>     - x is DU before the loop body*
>>     - x is DU after the 'break' (because this is unreachable)
>>     - x is DU after the loop body
>>
>>     - thus, x is DU before the (empty) condition
>>
>>     - x is DU before 'x=2'
>>
>>     - x is DU after the 'break'
>>
>>     - x is DU after the loop body
>>
>>     - x is DU before the incrementation statement
>>
>>     - x is DU before 'x=1'
>>
>>     (*The connection between the start of the condition and the start
>>     of the loop body is a little confusing in the spec, because when
>>     the condition is empty, it refers to the initialization instead.
>>     But you get the same answer either way.)
>>
>>     So, the compiler is wrong: there is no error in Sample1.
>>
>>>     public class Sample2 {
>>>      final int x;
>>>
>>>      Sample2() {
>>>        x=2;
>>>        for(;;x=1) {
>>>          break;
>>>        }
>>>      }
>>>     }
>>
>>     No error here, which is fine. 'x=1' is unreachable, so it doesn't
>>     matter that it performs an assignment.
>>
>>     (You get the same behavior if you do 'if (false) x = 1'.)
>>
>>     —Dan
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20180216/93fac571/attachment.html>