netbeans icon indicating copy to clipboard operation
netbeans copied to clipboard

Revert PR4025 due to changes in compile time constants.

Open neilcsmith-net opened this issue 1 year ago • 3 comments

Straight revert of #4025 due to breaking changes in compile time constants.

cc @lkishalmi @mbien @BradWalker alternative option to revert rather than patch. Some of the other changes could then be looked at and reviewed again in master for NB16.

neilcsmith-net avatar Aug 12 '22 08:08 neilcsmith-net

Good morning folks. Hey @neilcsmith-net , @mbien can I get a clearer picture of what is broken? The bug says "breaking changes" but it's not clear to me what is broken.. Clearly the platforms are dead, so maybe I can easily fix it instead of doing a revert..

BradWalker avatar Aug 12 '22 12:08 BradWalker

@BradWalker values of public final primitive fields can't change. If they do it will break existing code since compilers can inline those sometimes. It would be also safer to simply keep all public fields and deprecate them. see #4497 and dev list.

mbien avatar Aug 12 '22 12:08 mbien

I'm not sure it's "can inline" but rather "must inline". eg. from JLS "If a final field is initialized to a constant expression in the field declaration, ... uses of that final field are replaced at compile time with the value of the constant expression".

@BradWalker this means that the value those fields are assigned should not change. Code (eg. third-party plugins) compiled against older versions of the IDE could have different values. The masks are also constant expressions. It's a binary incompatible change.

The values are also in the API signature files - I saw it initially when generating those for NB15. Check the diff in #4487 The tests should really have not passed with the changes.

I'm of the opinion that revert, fix, re-apply is sometimes a better strategy for handling breaking changes. Obviously here the re-apply would be in master for NB16 though.

neilcsmith-net avatar Aug 12 '22 13:08 neilcsmith-net