9 ms·
Java: "+=" applied to String operands can provoke side effects
- Terr_ 8y ago> Operator += is broken for Strings in Java 9 and 10 The title [Edit: since changed] is a little bit fear-monger-y. It's only broken when the left hand side is not safe to evaluate more than once. It can be prevented with a compile-time flag.
- reificator 8y agoI mean, that's fairly broken given the way developers expect operators to be applied. The ease of avoidance is unrelated to whether it's broken behavior or not.
- gowld 8y agoIt's a bug for sure, but anyone who cares about correctness ought to be using a style that doesn't allow this tortured contortion: array[i++] += i ...
- esrauch 8y agoThe same bug repros for array[method()] += "string"; where method is invoked twice instead of once.
- simias 8y agoIf I understand correctly "array[i++] += ..." alone is enough to trigger the bug which doesn't seem particularly tortured to me. Actually any "array[side_effect()] +=" would trigger the bug. That's pretty bad IMO.
- seandougall 8y agoEven if the function within the subscript were free if side effects, calling it twice would be a ridiculous performance regression. I’m no compiler developer, but this really seems like a rookie mistake.
- Dylan16807 8y agoDon't focus too much on the first example. array[i++] += "a" is just is broken. So is appending to array[foo()] or foo().field
- deleted 8y ago[deleted]
- java-man 8y agoI think it's a big deal: introduced in 9 and fixed in 11? It suggests two things: Oracle has gaps in the regression test suite, and they don't consider compiler error such as these as a big issue. Java 9 broke other things. Here is one: https://github.com/andy-goryachev/FxDock/issues/8 https://github.com/andy-goryachev/FxDock/issues/8 WebView is working fine in 8, totally broken in 9, partially broken in 10. Do they have a QA department?
- kjeetgill 8y agoYea, I'm surprised the infamous TCK regression suite didn't include something so fundamental. That said, javafx is a bit of an orphan compared the rest of the JDK. And that's why it's getting pushed out.
- java-man 8y agothat's another gripe...
- kjeetgill 8y agoI don't know if this is your issue but in a minor Java 8 release, the JavaFX team made all references held by a webview's JS weak references. What a dramatic change over what seems to be a solvable memory leak. I only toyed with JavaFX but at least it's webview is dead to me. See: https://bugs.java.com/view_bug.do?bug_id=8154127 https://bugs.java.com/view_bug.do?bug_id=8154127
- java-man 8y agointeresting. no, it is not my issue - in my case nothing gets rendered in WebView, the view stays blank. and another thing - how does one submit a defect report to Oracle? neither bugs.openjdk.java.net nor bugs.java.com have a clear link, and I can't find where one can register an account.
- gurkendoktor 8y agoJavaFX HiDPI support on Windows also underwent breaking changes in minor Java 8 updates. It's probably for the best that JavaFX can have its own, independent version number now that it was kicked out of the JDK.
- kjeetgill 8y agoIt's easy to work around, sure, but it's still going to be something that's hard to catch if you see it in the wild. Also: it only breaks when it's an array of Strings.
- rhacker 8y agoNot so sure about that... let's say this bug is causing a minor inconsistency in any one of the thousands of databases built on top of Java, not the least one of the most popular databases Elasticsearch. We may not know yet, but this bug could theoretically be causing different output. In fact if we see anything from the Lucene project related to this bug, then we know for sure ES and other products are likely affected.
- sctb 8y agoWe've updated the link from https://stackoverflow.com/q/50683786 https://stackoverflow.com/q/50683786 and the title from “Operator += is broken for Strings in Java 9 and 10” to the official issue page.
- oblong 8y ago> It's only broken when the left hand side is not safe to evaluate more than once. The brakes on my car aren't broken most of the time, it's only when I push down on the pedal!
- kjeetgill 8y agoWow. This is a pretty big bug. The official bug report: https://bugs.openjdk.java.net/browse/JDK-8204322 https://bugs.openjdk.java.net/browse/JDK-8204322 as Stuart Marks, a jvm architect, posted. I'm surprised it stayed uncaught for so long. For the unfamiliar, the way string concatenation is compiled changed with invokedynamic. Previously, it would compile to either Sting.concat calls or transform into a StringBuilder, but with invoke dynamic we can leave it to the runtime to optimize when it first runs.
- lern_too_spel 8y agoIs it really surprising? Nobody is running production workloads on Java 9 pr 10 because neither of them are LTS releases.
- dionian 8y agoDoes this only apply to shops that cannot easily upgrade to 12 once it's out? Or are there other support-related problems with adopting 9/10 for production for server-side apps?
- IloveHN84 8y agoWe are using it in production. Even if it isn't LTS, does it make sense to.stay on the 8, which will be end of life soon anyway? At least, programming using modules makes the codebase ready for Java 11. The only thing that I don't like is the removal of javapackager without substituting it (jlink cannot produce bundles with applications, only a light redistributed image)
- haglin 8y agoI believe there is a JEP to fix that, but it doesn't say if it will make JDK 11. http://openjdk.java.net/jeps/8200758 http://openjdk.java.net/jeps/8200758
- mabn 8y agoIf you upgrade to 9 and then to 12 then you'll need to do this dance twice..
- lcfcjs 8y agoHow can something as simple as string concatenation bugs not get caught in regression testing? (Or even Unit testing)? No wonder everybody is moving away from Java.
- nine_k 8y agoAdding side effects to your array index expression and depending on order of their evaluation is a bad idea anyway. Unfortunately, the `++` / `--` made it to Java, and are still used sometimes, despite JVM not being PDP-11, and definitely not trying to conserve every precious byte of RAM.
- wrs 8y agoApparently it’s nothing specific about arrays or ++ — any expression on the left hand side of the compound operator is evaluated twice, so any side effects happen twice.
- nine_k 8y agoIf you read the original post to the end, you'll notice a piece of code that seemingly refutes this statement. That is, in case of Java 8, the index expression is evaluated once. and in case of Java 9, twice.
- Dylan16807 8y agoWhat does that refute? You seem to be agreeing with the post you're replying to. But importantly your original point about "depending on order of their evaluation" is wrong. Most of the examples don't care about order at all. They break because things happen the wrong number of times.
- asdsa5325 8y agoRead his comment again.
- kjeetgill 8y agoIt's worth noting: the bug in question only occurs when it's an array of String[]. Strings are a special case for just about everything in the compiler. :(
- cesarb 8y agoUnlike in C or C++, the order of evaluation in Java is well defined, so there's no problem depending on it.
- ianamartin 8y agoTime to rewrite Java in Rust.
- dtech 8y agoI know it's a (bad) joke, but Rust wouldn't solve this. This is incorrectly implemented logic and Rust will not prevent you from doing that.
- kibwen 8y agoTo be marginally less facetious, Rust would "fix" this problem by dint of deliberately neither having a ++ operator (in either post- or pre-increment varieties) nor by allowing assignment operators (including augmented assignment ops) to evaluate to the value of their RHS, so the example from the OP would have to be written in a different fashion. :P (Rust has previously had bugs of its own related to the evaluation order of += and friends, it seems like a problem that nearly every language has at some point...)
- Dylan16807 8y ago> neither having a ++ operator The bug happens with a function call on the LHS too. > nor by allowing assignment operators (including augmented assignment ops) to evaluate to the value of their RHS I'm confused. The assignment is happening last, and whatever it evaluates to is being discarded. How would changing what it evaluates to do anything? But more importantly, the comment was about implementing Java itself in Rust, not the broken function, and that is why it wouldn't change anything.
- kjeetgill 8y agoIt'll never happen. They're too busy rewriting Java in Java! See: substrateVM, graal
- pleasecalllater 8y agoYea... what was the Bloch's explanation for not having operator overloading in Java? I think it was something like "it's not allowed because I've seen too many programmers who did that in a wrong way" :) On the other hand this explanation is stupid, as a programmer can implement `add()` in a wrong way.
- kazinator 8y agoLisp macros get this right. [2]> (macroexpand '(incf (aref a (incf i)) x)) (LET* ((#:G3214 A) (#:G3215 (INCF I)) (#:G3217 X) (#:G3216 (+ (AREF #:G3214 #:G3215) #:G3217))) (SYSTEM::STORE #:G3214 #:G3215 #:G3216)) ; T You can't just blindly expand the += by copying the expressions/AST node on the left. Or in general; you don't take expressions in the program and blindly duplicate them. How can someone working on compilers not know that???
- kjeetgill 8y agoI see what you're getting at, but that's not the mistake that was made here. People working on it knew what it should do. It's in the spec so Java agrees with you. This bug is a little narrower than this makes it out to be, it ONLY fails when the array on the left is a String[ ]. The way String operations get compiled down to bytecode changed a lot around those releases so the bug is more like an unforeseen interaction between optimization passes than a naive MST/Macro implementation. The "funny" thing here to me is the surprising gap in the regression testing given the quality of most of the JDK. go figure
- oblong 8y ago> "side effects" A side effect would be like submitting an order for am Amazon Echo by mistake. This is a bug, a serious bug, that leads to variables being assigned the wrong value.
- freefal 8y agoI used that operator in a tight loop before the compiler was smart enough to automatically convert the code to use a StringBuilder. Boy was that slow...
- rmetzler 8y agoI didn't touch much Java Code for ~ 10 years now. I remember reading about the string concatenation problem in the Effective Java book. Using a StringBuilder was the very first thing I usually tried when I needed to improve performance on some unfamiliar code. I didn't know the Java compiler converts this automatically now. Since which version is this? I suspect there might be still edge cases where it doesn't work.