770

For a challenge, a fellow code golfer wrote the following code:

import java.util.*;
public class Main {
  public static void main(String[] args) {
    int size = 3;
    String[] array = new String[size];
    Arrays.fill(array, "");
    for(int i = 0; i <= 100; ) {
      array[i++%size] += i + " ";
    }
    for(String element: array) {
      System.out.println(element);
    }
  }
}

When running this code in Java 8, we get the following result:

1 4 7 10 13 16 19 22 25 28 31 34 37 40 43 46 49 52 55 58 61 64 67 70 73 76 79 82 85 88 91 94 97 100 
2 5 8 11 14 17 20 23 26 29 32 35 38 41 44 47 50 53 56 59 62 65 68 71 74 77 80 83 86 89 92 95 98 101 
3 6 9 12 15 18 21 24 27 30 33 36 39 42 45 48 51 54 57 60 63 66 69 72 75 78 81 84 87 90 93 96 99 

When running this code in Java 10, we get the following result:

2 4 6 8 10 12 14 16 18 20 22 24 26 28 30 32 34 36 38 40 42 44 46 48 50 52 54 56 58 60 62 64 66 68 70 72 74 76 78 80 82 84 86 88 90 92 94 96 98 
2 4 6 8 10 12 14 16 18 20 22 24 26 28 30 32 34 36 38 40 42 44 46 48 50 52 54 56 58 60 62 64 66 68 70 72 74 76 78 80 82 84 86 88 90 92 94 96 98 100 102 
2 4 6 8 10 12 14 16 18 20 22 24 26 28 30 32 34 36 38 40 42 44 46 48 50 52 54 56 58 60 62 64 66 68 70 72 74 76 78 80 82 84 86 88 90 92 94 96 98 100 

The numbering is entirely off using Java 10. So what is happening here? Is it a bug in Java 10?

Follow ups from the comments:

  • The issue appears when compiled with Java 9 or later (we found it in Java 10). Compiling this code on Java 8, then running in Java 9 or any later version, including Java 11 early access, gives the expected result.
  • This kind of code is non-standard, but is valid according to the spec. It was found by Kevin Cruijssen in a discussion in a golfing challenge, hence the weird use case encountered.
  • Didier L found out that the issue can be reproduced with the much smaller and more understandable code:

    class Main {
      public static void main(String[] args) {
        String[] array = { "" };
        array[test()] += "a";
      }
      static int test() {
        System.out.println("evaluated");
        return 0;
      }
    }
    

    Result when compiled in Java 8:

    evaluated
    

    Result when compiled in Java 9 and 10:

    evaluated
    evaluated
    
  • The issue seems to be limited to the string concatenation and assignment operator (+=) with an expression with side effect(s) as the left operand, like in array[test()]+="a", array[ix++]+="a", test()[index]+="a", or test().field+="a". To enable string concatenation, at least one of the sides must have type String. Trying to reproduce this on other types or constructs failed.

idmean
  • 13,418
  • 7
  • 47
  • 78
Olivier Grégoire
  • 28,397
  • 21
  • 84
  • 121
  • 5
    Comments are not for extended discussion; this conversation has been [moved to chat](https://chat.stackoverflow.com/rooms/172457/discussion-on-question-by-olivier-gregoire-why-does-arrayin-i-give). – Samuel Liew Jun 04 '18 at 23:22
  • This seems like it could easily be a problem in "real" code. If I understand correctly, something like `array[i++] += n` is enough to trigger `i++` twice? – JollyJoker Jun 05 '18 at 08:08
  • 13
    @JollyJoker It is limited to `+=` applied to indirect `String` references. So first, your array must be a `String[]`. The issue doesn't occur with `int[]`, `long[]` and friends. But yes, you're basically right! – Olivier Grégoire Jun 05 '18 at 08:11
  • 2
    @OlivierGrégoire the array doesn’t need to be `String[]`. If it is `Object[]` and you do `array[expression] += "foo";`, it’s the same. But yes, it doesn’t apply to primitive arrays, as it must be able to hold references of type `String` (`Object[]`, `CharSequence[]`, `Comparable[]`, …), to store the result of string concatenation. – Holger Jun 05 '18 at 09:29
  • Why can't I reproduce it in jdoodle? https://www.jdoodle.com/a/wSU – danadam Jun 05 '18 at 14:09
  • @danadam Because the code is compiled with a Java 8 VM or something like that: the Java-10-keyword `var` is not recognized (such as in `var x = "";`. – Olivier Grégoire Jun 05 '18 at 14:17
  • 33
    This has been assigned bug id [JDK-8204322](https://bugs.openjdk.java.net/browse/JDK-8204322). – Stuart Marks Jun 05 '18 at 15:19
  • 1
    @StuartMarks thanks! That's been integrated in the answer: I really wanted to keep the question a question, about whether it's normal or a bug. Though, we could be more explicit about the ID of the bug in the answer. I'll adapt it right now. – Olivier Grégoire Jun 05 '18 at 15:55
  • 1
    What do you mean by "non-standard" code? – xehpuk Jun 08 '18 at 15:25
  • 1
    @xehpuk statements with several effects / side-effects. – Olivier Grégoire Jun 08 '18 at 15:29
  • 1
    Things like this are making me believe that "Oracle is not able to handle Java development properly" – nomadSK25 Jun 04 '19 at 10:42
  • Very interesting bug, thanks. We have already fixed it in JDK11 – Vicente Romero Jun 06 '18 at 20:56
  • Thank you @VicenteRomero for having fixed this bug this quick! When will the backport be publicly available for Java 10? – Olivier Grégoire Jun 06 '18 at 22:07
  • 1
    @OlivierGrégoire, sorry for the late reply, just to complete the thread, the issue was backported to JDK 10 on 2018-06-06 https://bugs.openjdk.java.net/browse/JDK-8204873 – Vicente Romero Feb 26 '21 at 22:08

1 Answers1

636

This is a bug in javac starting from JDK 9 (which made some changes with regard to string concatenation, which I suspect is part of the problem), as confirmed by the javac team under the bug id JDK-8204322. If you look at the corresponding bytecode for the line:

array[i++%size] += i + " ";

It is:

  21: aload_2
  22: iload_3
  23: iinc          3, 1
  26: iload_1
  27: irem
  28: aload_2
  29: iload_3
  30: iinc          3, 1
  33: iload_1
  34: irem
  35: aaload
  36: iload_3
  37: invokedynamic #5,  0 // makeConcatWithConstants:(Ljava/lang/String;I)Ljava/lang/String;
  42: aastore

Where the last aaload is the actual load from the array. However, the part

  21: aload_2             // load the array reference
  22: iload_3             // load 'i'
  23: iinc          3, 1  // increment 'i' (doesn't affect the loaded value)
  26: iload_1             // load 'size'
  27: irem                // compute the remainder

Which roughly corresponds to the expression array[i++%size] (minus the actual load and store), is in there twice. This is incorrect, as the spec says in jls-15.26.2:

A compound assignment expression of the form E1 op= E2 is equivalent to E1 = (T) ((E1) op (E2)), where T is the type of E1, except that E1 is evaluated only once.

So, for the expression array[i++%size] += i + " ";, the part array[i++%size] should only be evaluated once. But it is evaluated twice (once for the load, and once for the store).

So yes, this is a bug.


Some updates:

The bug is fixed in JDK 11 and was back-ported to JDK 10 (here and here), but not to JDK 9, since it no longer receives public updates.

Aleksey Shipilev mentions on the JBS page (and @DidierL in the comments here):

Workaround: compile with -XDstringConcat=inline

That will revert to using StringBuilder to do the concatenation, and doesn't have the bug.

Olivier Grégoire
  • 28,397
  • 21
  • 84
  • 121
Jorn Vernee
  • 26,917
  • 3
  • 67
  • 80
  • 35
    By the way, this applies to the entire left hand side expression, not only the index providing sub-expression. This expression may be arbitrarily complex. See for example `IntStream.range(0, 10) .peek(System.out::println).boxed().toArray()[0] += "";`… – Holger Jun 05 '18 at 09:45
  • 10
    @Holger The left hand side does not even need to involve arrays, the issue also occurs with a simple `test().field += "sth"`. – Didier L Jun 05 '18 at 11:40
  • 2
    @DidierL I know. I already tested that. I just didn’t find a simple example fitting into a comment, as there’s no JRE class having an appropriate public field. I already wrote a comment referring to `+=` in general (if it’s a string concatenation), but apparently, it has been removed because of a sarcastic additional sentence. By the way, the field even might be `static`, making the redundant evaluation even less unnecessary. – Holger Jun 05 '18 at 11:46
  • 44
    Not that it matters, the behavior is horribly broken anyway, but the first evaluation is for the store and the second for the load, so `array[index++] += "x";` will read from `array[index+1]` and write to `array[index]`… – Holger Jun 05 '18 at 12:08
  • As per bug report, direct fix version is 11 and it'll be backported to 10. So there won't be any backports for 9? – The Coder Jun 06 '18 at 07:47
  • 5
    @TheCoder Yeah I think so. JDK 9 is not a long term support (LTS) release. JDK 8 was, and the next LTS release is JDK 11. See here: http://www.oracle.com/technetwork/java/javase/eol-135779.html Note that public updates to JDK 9 ended in March. – Jorn Vernee Jun 06 '18 at 08:37
  • 2
    @Holger wow, that's really bad, I'm surprised no was saw this before and there were no tests for it. – agilob Jun 06 '18 at 11:44
  • 1
    Not that of complaining. If the fix won't be backported to JDK 9, then practically no one should run their apps JDK 9 :( – The Coder Jun 06 '18 at 14:40
  • 1
    @TheCoder if you upgraded to Java 9, you had to upgrade to Java 10 as soon as it was out since it signed the end of public updates for Java 9. Same will be with 10 and 11. 11 will be LTS so it should be quite typical to jump from 8 to 11. – Didier L Jun 06 '18 at 19:24
  • 15
    On JDK-8204322, Aleksey Shipilev suggested to compile with `-XDstringConcat=inline` as a workaround, for those who need it. – Didier L Jun 06 '18 at 19:50
  • 3
    @DidierL [Can indeed confirm the compiler-flag works for the provided code of _@OlivierGrégoire_ in TIO Java 10](https://tio.run/##TY2xTsMwEIb3PMUpElJCqFXERsiA6NqpC1KVwaROdME5R/alqEJ5dYwbt5Tp7P//7rteHuWqP3x6j8NoLEMfAjExanFfJsk4fWhsoNHSOdhKJPhOAC6pY8lhHA0eYAhdtmOL1O1rkLZz@YICIDEQVPBULt9/jJWnkJP6uoZUR@b1XDnRotbZgj1Amuaxa43NzkoMq@syjJcKHtfhdb0H0bzHorijGooqMAWkkMb9@c8Sj4LSalDEz3HtZtmdHKtBmInFGEDWlF3Q/Gaak9n7n6bVsnN@9b5xi/PNUCO5QtJI6hc). – Kevin Cruijssen Jun 08 '18 at 11:57