Handling for new warning?
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Wed Sep 25 01:37:07 UTC 2019
My (very humble) 0.02$ on this; I can see where Mike is coming from
_and_ where Jon and Mark are coming from.
The problem is that, when we are inside an _implementation_ class (think
of a non-exported package in a module), we probably don't want to be too
formal about the set of unchecked annotations being thrown.
Conversely, when we are in a public-facing API, it is very likely that
unchecked exceptions carry value (IllegalArgumentExecption,
IllegalStateException to name a few) and should therefore be documented.
To be more concrete, on my recent work on the memory access API, while
developing the API and writing the docs I wish I had an automatic way to
detect all unchecked exception I missed in the 'throws' clause, as they
were likely (a) some documentation missing somewhere or (b) a missing
test for that particular failure mode.
The problem is how to reconcile these two aspects - you want a warnings
that's useful, but not one that's excessively punitive so that
apparently good code gets flagged (I am reminded of when I introduced
the warning for raw types and later on relaxed that not to be triggered
on stuff like 'instanceof List', which is pretty widespread - and for
which adding an extra '<?>' doesn't really add a lot of value).
And, in this case, it can be tricky to come up with a way to distinguish
between API and internals. Perhaps we could use the module system to our
advantage, and only enable the Lint check for public methods in public
classes/interfaces in exported packages. I'm pretty sure something like
this should reduce the 'noise' of such a check quite significantly (and
the remaining places where the warning is still issued are likely to be
informative for us).
Thoughts?
Maurizio
On 25/09/2019 01:59, Jonathan Gibbons wrote:
>
> On 9/24/19 5:46 PM, Mike Duigou wrote:
>>
>>
>>
>>
>> On 2019-09-24 16:25, mark.reinhold at oracle.com wrote:
>>> 2019/9/24 15:56:37 -0700, openjdk at duigou.org:
>>>> The proposed change is to add a lint level warning, ie.
>>>> -XLint:runtime-undeclared , suggestion to add the unrequired but
>>>> missing
>>>> throws declarations for RuntimeException et al. The advantage to
>>>> providing the complete throws including RuntimeExceptions is to make
>>>> them more visible and encourage handling of them. We have one good
>>>> example of a library which does declare the RuntimeException that are
>>>> thrown, the JDK.
>>>
>>> I’m confused. I thought you said earlier that there are thousands of
>>> missing cases in the JDK.
>>
>> JDK seems to be in pretty good shape though -Werror stops compilation
>> well before completion. The observed problems with large numbers of
>> missing declarations have been in javac where there are hundreds
>> (perhaps many more) missing throws declarations primarily, so far,
>> for IllegalArgumentException and CompletionFailure.
>
> I don't think it would be acceptable to change javac so that
> CompletionFailure is specified all over the place. Story: a long
> time ago, back when I joined the javac team, I tried doing something
> similar to what you're proposing now, except that I tried to do it by
> changing CompletionFailure to a checked exception. As you might guess,
> the size of the change became unreasonable and I abandoned the
> experiment.
>
> I'd also be concerned about IllegalArgumentException and friends like
> NullPointerException becoming too pervasive across the codebase.
>
> -- Jon
>
>
>>
>> There's no easy way to know the total number of potential throws
>> "fixes" needed because adding missing declarations has a tendency to
>> produce new warnings in the methods which call the fixed method. I
>> added a couple hundred across a dozen or so files but didn't feel I
>> was any near complete based on the number of files I hadn't yet
>> covered in the javac source.
>>
>>>
>>>> ...
>>>>
>>>> The check will apply to all instances of RuntimeException and all
>>>> subclasses.
>>>
>>> I agree with Jon: This proposal is likely to have broad impact and thus
>>> needs wider discussion. May I suggest the jdk-dev list?
>>
>>
>> For JDK this would seem to be a separate problem--the missing
>> "throws" could in some cases mean that the method signature and
>> javadoc are an incomplete specification of the API and a
>> potential/probable JCK issue.
>>
>> For the moment I am going to working on completing the minimal patch
>> which provides the new lint warning and allows compilation by
>> disabling the warning.
>>
>>>
>>> - Mark
>>
>>
>> Cheers,
>>
>> Mike
More information about the compiler-dev
mailing list