[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-compiler-dev
Subject:    Re: RFR JDK-8164836: fix jdeprscan violations of tools/all/RunCodingRules.java
From:       Kumar Srinivasan <kumar.x.srinivasan () oracle ! com>
Date:       2016-08-29 17:35:14
Message-ID: 57C47252.7020201 () oracle ! com
[Download RAW message or body]


+1, and thank you so much Jan for taking care of javadoc.

Kumar

> Looks OK to me.
>
> -- Jon
>
> On 08/28/2016 01:31 PM, Jan Lahoda wrote:
>> On 26.8.2016 21:02, Jonathan Gibbons wrote:
>>> Arguably, we should only check @DefinedBy in java.compiler and
>>> jdk.compiler modules.
>>
>> Updated version of the patch where the DefinedByAnalyzer only checks 
>> the java.compiler and jdk.compiler modules, and the @DefinedBy 
>> annotation is removed from the other modules is here:
>> http://cr.openjdk.java.net/~jlahoda/8164836/webrev.01/
>>
>> Jan
>>
>>>
>>> -- Jon
>>>
>>> On 08/26/2016 08:36 AM, Stuart Marks wrote:
>>>> Hi Jan,
>>>>
>>>> Thanks for picking this up.
>>>>
>>>> As you point out, the API usage in jdeprscan, and its containing
>>>> module jdk.jdeps, really isn't the target of @DefinedBy. It's
>>>> "implementing" the APIs, but for the purpose of getting callbacks from
>>>> the annotation processing framework, not for the purpose of providing
>>>> an implementation of the API to the general public. In that sense
>>>> @DefinedBy doesn't really apply, so it makes sense to adjust the
>>>> DefinedByAnalyzer to be more specific in what it's checking.
>>>>
>>>> But does special-casing jdk.jdeps make sense? Or should there be some
>>>> kind of finer-grained logic to determine when @DefinedBy should and
>>>> shouldn't apply? I wouldn't be surprised if there are other usages of
>>>> @DefinedBy outside of jdk.jdeps that are in a similar situation. See
>>>> jdk.jshell for instance.
>>>>
>>>> I've prepared a patch -- not yet posted anywhere -- that simply adds
>>>> the right @DefinedBy annotations in jdeprscan. This is simple enough
>>>> to do, even though it might not be the right thing.
>>>>
>>>>
>>>>
>>>> s'marks
>>>>
>>>>
>>>>
>>>>
>>>> On 8/26/16 5:00 AM, Jan Lahoda wrote:
>>>>> Hi,
>>>>>
>>>>> The RunCodingRules test is broken after 871b60b0c091, and has been
>>>>> disabled. My
>>>>> proposal to fix this is to disable the DefinedByAnalyzer for classes
>>>>> from the
>>>>> jdk.jdeps module. This module is not AFAIK implementing the APIs, and
>>>>> so is not
>>>>> a primary target of the analyzer/@DefinedBy annotation. One notable
>>>>> alternative
>>>>> would be to disable the crules completely for jdk.jdeps, but I opted
>>>>> rather for
>>>>> a more limited change. (Another alternative would be to add the
>>>>> @DefinedBy
>>>>> annotations to classes jdk.jdeps, but that does not seem quite right
>>>>> to me.)
>>>>>
>>>>> Bug:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8164836
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~jlahoda/8164836/webrev.00/
>>>>>
>>>>> Jan
>>>
>

[prev in list] [next in list] [prev in thread] [next in thread]