RFR: 8223553: Fix code constructs that do not compile with the Eclipse Java Compiler

[Stuart Marks]

On 5/23/19 6:39 AM, Langer, Christoph wrote:
> big thanks for your great updates here. This all looks a lot cleaner: http://cr.openjdk.java.net/~clanger/webrevs/8223553.3/

Great, glad to help. While I'm still unsure about the underlying reasons for 
this disagreement between the compilers, that might take a while to resolve. 
Putting in these fixes isn't very intrusive and it seems to make people's lives 
easier, so I don't have any objection to them going in.

> In the other 3 locations, we're able to eliminate the EC4J issues by introducing a local variable with the right type declaration. Sounds like a viable workaround.
> 
> At Eclipse we have the following bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=530236. It refers to the OpenJDK bug https://bugs.openjdk.java.net/browse/JDK-8016207.
> 
> I'm wondering whether this should be submitted and I should at the same time submit another bug to evaluate these code places at a time when the final resolution for JDK-8016207 was provided?

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.

I'll defer to Martin as to how he wants to handle the ConcurrentSkipListMap.java 
change.

s'marks