> Overall, introducing a local variable is more-or-less reasonable even if it's > used only once. One point is that somebody might come along and "clean > up" the > code and inline local variables and reintroduce the problem. Another point is > that, hypothetically, if in the future Eclipse is changed to match javac's > behavior, these changes should be reverted. > > The bug database is a good place to capture actions that need to take place in > the future. Unfortunately, it's pretty far from these locations, so the > existence of such a bug wouldn't prevent the accidental cleanup from > happening. > That would indicate having a comment in the code at these locations. > > On the other hand, if Eclipse is "fixed" and these locations don't get cleaned > up for a long time, it doesn't seem like that big a deal. > > I'd suggest to put a comment at these 3 locations, something like: > > // local variable required here; see JDK-8223553 > > and not bother with filing another bug report to track this.
Ok, good idea. I'll add the comment before I push. > I'll defer to Martin as to how he wants to handle the > ConcurrentSkipListMap.java > change. As Martin has taken this to the jsr166 integration 2019-06, I'll push the change without ConcurrentSkipListMap.java tomorrow. Thanks to all involved in this review! /Christoph
