RFR: JDK-8223914: specification of j.l.c.MethodTypeDesc::of should document better the exceptions thrown
Roger.Riggs at oracle.com
Wed May 22 17:42:40 UTC 2019
On 05/20/2019 03:29 PM, Vicente Romero wrote:
> Hi Roger,
> On 5/20/19 2:00 PM, Roger Riggs wrote:
>> Hi Vicente,
>> In the CSR, the Summary should be about the change...
>> "...MethodTypeDesc::of should document all exceptions.
>> Avoid duplication between Summary and Problem.
> thanks, I saw that you already modified the Summary
>> I would omit the part about "content of parameter" or "its contents"
>> is null;
>> It cannot happen and if it does, its more of an internal error than a
>> regular NPE
>> since it should not be possible to create a ClassDesc with a null
>> descriptor string.
> this spec comment was included in this one an other similar methods.
> It is probably a bit of an overkill but we have already changed the
> spec of several similar methods to this pattern, see for example 
Its just a bad to propagate a poor spec as it is to write new bad spec.
>> In the Code Review:
>> MethodTypeDesc.java: 68: as above, "or its contents are" -> "is "
>> there's no need to mention the contents.
>> MethodTypeDescTest.java: 264 and 274. The messages would be more
>> useful (if they ever were to happen)
>> and for the person reading the code if they describe what should not
>> For example, "ClassDesc array should not be null" or ClassDesc
>> should not be null.
> I made this change in the test comment 
>> Thanks, Roger
>  https://bugs.openjdk.java.net/browse/JDK-8223803
>  http://cr.openjdk.java.net/~vromero/8223914/webrev.01/
>> On 05/17/2019 12:55 PM, Vicente Romero wrote:
>>> Please review simple fix for  at  plus the CSR at . This
>>> fix is simply documenting all the missing cases in which method
>>> java.lang.constant.MethodTypeDesc::of can throw exceptions. A test
>>> has been added to cover the missing cases.
>>>  https://bugs.openjdk.java.net/browse/JDK-8223914
>>>  http://cr.openjdk.java.net/~vromero/8223914/webrev.00/
>>>  https://bugs.openjdk.java.net/browse/JDK-8224136
More information about the core-libs-dev