[9] RFR (S): 8157181: Compilers accept modification of final fields outside initializer methods

[Coleen Phillimore]

On 6/14/16 10:17 AM, Vladimir Ivanov wrote:
>
>>>>> Also, have you run the JCK tests?
>>>>>
>>>>> Why have a global flag if it's an error?
>>>>
>>>> We are enforcing a rule that has not been enforced before. To deal 
>>>> with
>>>> compatibility issues we have provided, as is customary for 
>>>> non-security
>>>> issues, a flag that allows the enforcement to be turned off, if it
>>>> causes a problem. (Though that is more likely needed for JDK 7/8
>>>> compatibility than for 9).
>>>
>>> FTR I'd like to see the checks turned ON by default for 7/8 in 9 and 
>>> the
>>> flag providing an escape hatch for compatibility purposes.
>>>
>>> It is motivated by the desire to limit possibilities of final field
>>> updates at runtime, so JIT-compilers have more freedom optimizing loads
>>> from final fields. It will be unfortunate if we effectively forbid 
>>> final
>>> field updates outside of initializers starting only from 9 (class files
>>>> =53) and not 7 (cf >=51).
>>>
>>> The change requires a CCC request and will be investigated separately
>>> (JDK-8159215 [1]).
>>
>> Ok. JDK 7/8 code exists now and by changing the rules you may break code
>> that was working. We can't expect that code to be fixed, but that is why
>> we have to provide the flag to disable the check. The question is
>> whether this will impact a large or small number of users, and whether
>> it will impact things that can readily have the flag applied.
>
> Yes, impact is what should drive the decision.
>
> The plan is to have the checks (either ON or OFF by default) only in 9 
> (no backports to 7/8).
>
> If the checks are ON by default, users should handle any problems 
> caused by them when migrating between major releases, which sounds 
> reasonable (if impact is low). There will be a choice to fix the 
> library/application or disable problematic checks (by setting the flag).
>
>>> If there are serious compatibility issues found during EA testing, we
>>> can turn the checks OFF by default.
>>
>> I doubt we will test very much that would expose this. As I understand
>> it this only affects non-Java languages running on the JVM. (Though I
>> supposed hand-assembled Java code could also have this.)
>
> There's Quality Outreach program [1] (conducted by Adoption Group) and 
> many popular FOSS Java projects test OpenJDK 9 builds. So, the 
> projects involved should notice if the change affects them.
>
>>> The flag is declared as diagnostic, so I don't think we need to go
>>> through deprecation process (as we do for product flags) and just 
>>> remove
>>> it in the next(?) major release.
>>
>> I had not looked at the code and did not know the flag was marked
>> diagnostic - that doesn't seem right to me. This isn't about 
>> diagnostics.
>
> Good point. Maybe make it product and deprecate right from the 
> beginning then?
>
>> The issue with removing the flag is when you can reasonably expect any
>> existing "bad" code will be updated. I can't judge the scope/extent of
>> the problem - it may be trivially small, or may not.
>
> Thinking about that more, I don't see much value in the flag if the 
> checks are OFF for 7-8 (cf <53) by default. If we don't enable the 
> checks in 9, I'm sure we will never change that.
>
> I don't see any value in providing opt-out option for 9 (cf >=53). The 
> impact is limited: javac doesn't produce such code shape and bytecode 
> generators require a change to produce cf =53. New code should adhere 
> to JVMS in this particular case.
>
> So, if we decide that enabling the checks for 7-8 is too risky, we can 
> remove the flag.

I would vote for pre-deprecating or removing the flag.  We don't 
generally add flags for new spec enforcement unless we anticipate a lot 
of customers having problems with the new rule.  Just make sure the 
error message is really good though, so that when people hit this new 
rule, they'll know what to fix.

Coleen
>
> Best regards,
> Vladimir Ivanov
>
> [1] https://wiki.openjdk.java.net/display/quality/Quality+Outreach