JDK 15 RFR of JDK-8230771: Remove terminally deprecated constructors in java.base
Stuart Marks
stuart.marks at oracle.com
Tue Dec 10 00:14:40 UTC 2019
Hi Joe,
The revised webrev looks good.
Side note on whether a private constructor should be empty or should throw some
Throwable. There are actually two (or more) use cases for private constructors:
1) intending to prevent instances from being created other than through
well-controlled APIs, such as static factory methods; and
2) intending to prevent instances from being created at all.
In case 1) an empty constructor might be perfectly reasonable. In case 2) an
empty constructor might accidentally allow an instance to be created, if the
code is later modified, or possibly reflectively. Throwing an exception from
this constructor is a good indication of the intent that instances never be created.
Throwing AssertionError seems reasonable but it might not be idiomatically the
best exception type. However, I won't quibble about the exception type.
I think there probably are cases in the JDK where we use a private, empty
constructor in cases where no instances are ever intended to be created. (I know
of at least one, because I wrote it.) It might be worthwhile to modify these to
throw an exception instead.
s'marks
On 12/9/19 1:34 PM, Joe Darcy wrote:
> PS That should be
>
> http://cr.openjdk.java.net/~darcy/8230771.1
>
> Cheers,
>
> -Joe
>
> On 12/9/2019 12:39 PM, Joe Darcy wrote:
>> Updated webrev:
>>
>> http://cr.openjdk.java.net/~darcy/8230771.0/
>>
>> Found a class extending Modifier to the the static definitions; I replaced
>> this with a static import.
>>
>> To make sure a class isn't instantiated, I usually have it throw
>> AssertionError or some exception, although as you say that isn't strictly
>> necessary in all cases.
>>
>> Thanks,
>>
>> -Joe
>>
>> On 12/9/2019 10:27 AM, Mandy Chung wrote:
>>> It seems a bit overly cautious to throw AssertionError. JDK has many private
>>> no-arg constructor and it can be as simple as empty constructor. Just my
>>> preference.
>>>
>>> Mandy
>>>
>>> On 12/9/19 10:16 AM, Joe Darcy wrote:
>>>> Corrected patch:
>>>>
>>>> diff -r 153e5f76551d
>>>> src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
>>>> --- a/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
>>>> Mon Dec 09 23:00:13 2019 +0530
>>>> +++ b/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
>>>> Mon Dec 09 10:15:44 2019 -0800
>>>> @@ -43,8 +43,7 @@
>>>> /**
>>>> * Do not call.
>>>> */
>>>> - @Deprecated(forRemoval=true, since="14")
>>>> - public ConstantBootstraps() {}
>>>> + private ConstantBootstraps() {throw new AssertionError();}
>>>>
>>>> // implements the upcall from the JVM,
>>>> MethodHandleNatives.linkDynamicConstant:
>>>> /*non-public*/
>>>> diff -r 153e5f76551d
>>>> src/java.base/share/classes/java/lang/reflect/Modifier.java
>>>> --- a/src/java.base/share/classes/java/lang/reflect/Modifier.java Mon Dec 09
>>>> 23:00:13 2019 +0530
>>>> +++ b/src/java.base/share/classes/java/lang/reflect/Modifier.java Mon Dec 09
>>>> 10:15:44 2019 -0800
>>>> @@ -46,8 +46,7 @@
>>>> /**
>>>> * Do not call.
>>>> */
>>>> - @Deprecated(forRemoval=true, since="14")
>>>> - public Modifier() {}
>>>> + private Modifier() {throw new AssertionError();}
>>>>
>>>>
>>>> /**
>>>>
>>>> -Joe
>>>>
>>>> On 12/9/2019 9:38 AM, Joe Darcy wrote:
>>>>> Doh! Will correct. That is why I want to get the no-arg constructor added
>>>>> as a javac warning (JDK-8071961) ;-)
>>>>>
>>>>> Thanks all for catching this,
>>>>>
>>>>> -Joe
>>>>>
>>>>> On 12/9/2019 9:29 AM, Mandy Chung wrote:
>>>>>> Good catch! Daniel also pointed that out. I overlooked it. It needs to
>>>>>> add back a private no-arg constructor.
>>>>>>
>>>>>> Mandy
>>>>>>
>>>>>> On 12/9/19 9:18 AM, Victor Williams Stafusa da Silva wrote:
>>>>>>> If you remove the deprecated constructor, the compiler will add a default
>>>>>>> one. Wouldn't it be a better idea to make the deprecated constructor
>>>>>>> private and throwing an exception?
>>>>>>>
>>>>>>> Em seg., 9 de dez. de 2019 às 14:13, Mandy Chung <mandy.chung at oracle.com
>>>>>>> <mailto:mandy.chung at oracle.com>> escreveu:
>>>>>>>
>>>>>>> Looks good.
>>>>>>>
>>>>>>> Mandy
>>>>>>>
>>>>>>> On 12/8/19 10:58 AM, Joe Darcy wrote:
>>>>>>> > Hello,
>>>>>>> >
>>>>>>> > Please review this small API changes for JDK 15:
>>>>>>> >
>>>>>>> > JDK-8230771: Remove terminally deprecated constructors in
>>>>>>> java.base
>>>>>>> > CSR: https://bugs.openjdk.java.net/browse/JDK-8235548
>>>>>>> > webrev: http://cr.openjdk.java.net/~darcy/8230771.0/
>>>>>>> >
>>>>>>> > Patch below.
>>>>>>> >
>>>>>>> > Thanks,
>>>>>>> >
>>>>>>> > -Joe
>>>>>>> >
>>>>>>> > ---
>>>>>>> >
>>>>>>> old/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
>>>>>>>
>>>>>>> > 2019-12-08 10:56:14.223168685 -0800
>>>>>>> > +++
>>>>>>> >
>>>>>>> new/src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java
>>>>>>>
>>>>>>> > 2019-12-08 10:56:13.999168685 -0800
>>>>>>> > @@ -40,12 +40,6 @@
>>>>>>> > * @since 11
>>>>>>> > */
>>>>>>> > public final class ConstantBootstraps {
>>>>>>> > - /**
>>>>>>> > - * Do not call.
>>>>>>> > - */
>>>>>>> > - @Deprecated(forRemoval=true, since="14")
>>>>>>> > - public ConstantBootstraps() {}
>>>>>>> > -
>>>>>>> > // implements the upcall from the JVM,
>>>>>>> > MethodHandleNatives.linkDynamicConstant:
>>>>>>> > /*non-public*/
>>>>>>> > static Object makeConstant(MethodHandle bootstrapMethod,
>>>>>>> > ---
>>>>>>> old/src/java.base/share/classes/java/lang/reflect/Modifier.java
>>>>>>> > 2019-12-08 10:56:14.775168685 -0800
>>>>>>> > +++
>>>>>>> new/src/java.base/share/classes/java/lang/reflect/Modifier.java
>>>>>>> > 2019-12-08 10:56:14.555168685 -0800
>>>>>>> > @@ -44,13 +44,6 @@
>>>>>>> > */
>>>>>>> > public class Modifier {
>>>>>>> > /**
>>>>>>> > - * Do not call.
>>>>>>> > - */
>>>>>>> > - @Deprecated(forRemoval=true, since="14")
>>>>>>> > - public Modifier() {}
>>>>>>> > -
>>>>>>> > -
>>>>>>> > - /**
>>>>>>> > * Return {@code true} if the integer argument includes the
>>>>>>> > * {@code public} modifier, {@code false} otherwise.
>>>>>>> > *
>>>>>>> >
>>>>>>>
>>>>>>
>>>
More information about the core-libs-dev
mailing list