RFR: JDK-8223914: specification of j.l.c.MethodTypeDesc::of should document better the exceptions thrown

Vicente Romero vicente.romero at oracle.com
Mon May 20 19:29:05 UTC 2019


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 [1]

>
> 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 
> happen.
> For example,  "ClassDesc array should not be null"  or ClassDesc 
> should not be null.

I made this change in the test comment [2]
>
> Thanks, Roger

Thanks,
Vicente

[1] https://bugs.openjdk.java.net/browse/JDK-8223803
[2] http://cr.openjdk.java.net/~vromero/8223914/webrev.01/

>
>
> On 05/17/2019 12:55 PM, Vicente Romero wrote:
>> Please review simple fix for [1] at [2] plus the CSR at [3]. 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.
>>
>> Thanks,
>> Vicente
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8223914
>> [2] http://cr.openjdk.java.net/~vromero/8223914/webrev.00/
>> [3] https://bugs.openjdk.java.net/browse/JDK-8224136
>



More information about the core-libs-dev mailing list