On 10/08/2015 01:06 PM, Alexander Scherbatiy wrote: > > The fix looks good to me. > Thank.
> Note that there are usually necessary to have at least two reviewers > for a fix in Open JDK. I thought patches before feature complete needs only one reviewer. [4][5] But two reviewers are always better. > > Are you going to push the fix as part of other fixes for different > JDK areas or as a separate fix? > In the second case you need to file a new bug for it. Actually i am planing to collect feedback from all dependent mailing-list and than decide how to progress. -- Sebastian [4] http://openjdk.java.net/guide/changePlanning.html [5] http://openjdk.java.net/projects/jdk9/ > > Thanks, > Alexandr. > > On 10/7/2015 10:59 PM, Sebastian Sickelmann wrote: >> Please find the updated webrev at: >> >> http://cr.openjdk.java.net/~sebastian/5108778/macos/webrev.00/ >> >> For some general discussion on regression-tests for this please find the >> thread in discuss[0][1] and for the general suggestion to make more >> wrapper-type-constructors deprecated find [2] at core-libs-dev. >> >> [0] >> http://mail.openjdk.java.net/pipermail/discuss/2015-September/003804.html >> >> [1] >> http://mail.openjdk.java.net/pipermail/discuss/2015-October/003805.html >> [2] >> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-October/035642.html >> >> >> -- Sebastian >> >> On 10/06/2015 03:06 PM, Alexander Scherbatiy wrote: >>> On 10/3/2015 5:40 AM, Stuart Marks wrote: >>>> >>>> On 9/28/15 2:03 AM, Alexander Scherbatiy wrote: >>>>> Is it possible to use autoboxing in the fix instead of >>>>> Boolean.valueOf(someBooleanVariable)? >>>> Hi, >>>> >>>> These cases are all interesting because they end up requiring boxed >>>> Boolean values, whether explicitly or implicitly via autoboxing: >>>> >>>> 1. AquaTabbedPaneUI.java -- use of boxed Boolean as a tri-state (!) >>>> >>>> 2. CAccessibility.java -- return value from Callable<Boolean> >>>> >>>> 3. LWCToolkit.java -- value in Properties (like Map<Object,Object>) >>>> >>>> For #2 and #3 I think auto-boxing instead of valueOf() is just fine. >>>> I guess using Boolean.TRUE or Boolean.FALSE instead of true or false >>>> is ok, as it's a micro-optimization to prevent autoboxing from >>>> generating a call to Boolean.valueOf(true|false). I'm sure the JIT >>>> compiler will inline such calls, so this speeds up interpreted code a >>>> tiny bit at the expense of a little verbosity in the code. >>>> >>>> The explicit calls to Boolean.valueOf(some boolean expression) I >>>> think can simply be replaced with autoboxing in all cases. >>>> >>>> The weird one is in AquaTabbedPaneUI.java, which has >>>> >>>> protected Boolean isDefaultFocusReceiver = null; >>>> >>> I do not mean to change the isDefaultFocusReceivertype type to >>> boolean. It was just interesting are there pros and cons to use a >>> short version with autoboxing for assignment: >>> isDefaultFocusReceiver = defaultFocusReceiver != null && >>> defaultFocusReceiver.equals(component); >>> instead of the long version: >>> isDefaultFocusReceiver = Boolean.valueOf(defaultFocusReceiver != >>> null && defaultFocusReceiver.equals(component)); >>> >>> Thanks, >>> Alexandr. >>> >>>> Ugh! It would be nice to get rid of null as a marker for >>>> uninitialized state, but that might require too much analysis to show >>>> that the change would be correct. This is, I think, the only case >>>> where the code has to be explicit about dealing with a boxed Boolean >>>> object that might be null, as opposed to true or false. >>>> >>>> The only place that really has to do that is >>>> isDefaultFocusReceiver(), which checks for null up front. I'm not >>>> sure that using Boolean.valueOf() and Boolean.booleanValue() in the >>>> rest of the method are really helpful in denoting that this is a >>>> boxed, nullable Boolean though. So I'd switch to autoboxing here too. >>>> >>>> s'marks >>> > >
