The fix looks good to me.
Note that there are usually necessary to have at least two reviewers
for a fix in Open JDK.
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.
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
