Possible compiler bug: continue is skipped after or-condition

[Maurizio Cimadamore]

I'm with Bernard here - presumably, javac could have different code 
generation strategies to allow for more or less source code fidelity. In 
practice, the cost of doing this (and maintaining this moving forward) 
is relatively high. In addition, I'm skeptical of doing anything without 
taking a more holistic approach how which kind of concrete source to 
bytecode fidelity level do we want to allow with flags XYZ, noting that, 
currently, source vs. classfile mismatches originates not only in the 
backend (because of optimizations such as the one described in this 
thread), but also because of specific ways in which javac decides to 
'compress' its internal AST representation -- for instance, javac 
aggressively constant folds string concatenation expressions, if it 
realizes that all components of the concatenation are constant - which 
means no real concatenation will happen at runtime.

So, rather than focusing on this specific use case, I think it would be 
more useful to collect _a list_  of all such use cases, describe how the 
behavior e.g. with IDE debugger, and/or code coverage tool deviates from 
expectations, and then see if we start to see some patterns emerging.

Maurizio


On 17/02/2020 14:45, B. Blaser wrote:

> Roland,
>
> I believe the current behavior to skip line 7, as you described below,
> looks correct.
> However, we could probably generate something closer to the source
> code when enabling debug with "-g" as we did for lambda factorization
> [1], for example.
> Maybe someone else could comment on this but I'm not sure the gain
> would worth the pain as you could certainly put a breakpoint on line
> 5.
>
> Any feedback is welcome,
> Bernard
>
> [1] http://mail.openjdk.java.net/pipermail/amber-dev/2018-March/002819.html
>
> On Sat, 15 Feb 2020 at 23:28, Roland Illig <roland.illig at gmx.de> wrote:
>> On 08.02.2020 20:05, Roland Illig wrote:
>>> /* - */ public class Demo {
>>> /* - */     static String[] strings = {"1a", "1b", "2", "3"};
>>> /* - */
>>> /* - */     public static void main(String[] args) {
>>> /* 5 */         for (String s : strings) {
>>> /* 6 */             if (s.equals("1a") || s.equals("1b")) {
>>> /* 7 */                 continue;
>>> /* - */             }
>>> /* 9 */             if (s.equals("2")) {
>>> /* - */                 continue;
>>> /* - */             }
>>> /* - */             System.out.println(s);
>>> /* - */         }
>>> /* - */     }
>>> /* - */ }
>> I investigated the problem a bit and found the cause.
>>
>> The documentation for Code.pendingJumps says:
>>   > A chain for jumps to be resolved before the next opcode
>>   > is emitted. We do this lazily to avoid jumps to jumps.
>>
>> This "jumps to jumps" that the pendingJumps successfully avoids is
>> exactly the code I expect to be generated. The first jump to line 7 is
>> necessary to visit the breakpoint there, even if that jump is followed
>> directly by another jump to line 5.
>>
>> I stepped through the code using a debugger, and when I skipped over the
>> following two lines in Code.branch once, the code was generated exactly
>> as I had expected it from the beginning.
>>
>>       result = pendingJumps;
>>       pendingJumps = null;
>>
>> Commenting these lines out will of course have other side effects, but
>> at least it makes the breakpoint in line 7 work again as expected.