RFR: JDK-8186688 javax.lang.model.util.Elements.hides does not work correctly with interfaces

Vicente Romero vicente.romero at oracle.com
Fri Feb 23 21:06:28 UTC 2018


looks good to me,

Vicente

On 02/23/2018 03:02 PM, Jonathan Gibbons wrote:
> Updated webrev.  Made new method final, and fixed comment.
>
> Copyright in test files was correct, and corresponds to when the test 
> was written and published, albeit in JBS.
>
> Updated webrev: http://cr.openjdk.java.net/~jjg/8186688/webrev.01
>
> -- Jon
>
> On 02/23/2018 07:32 AM, Jonathan Gibbons wrote:
>> Maurizio,
>>
>> Thanks for the review.
>>
>> Yes, the intent was not to change the behavior of any existing uses 
>> of isInheritedIn in javac, except for the one problem case in 
>> JavacElements, where even the existing comment hinted that the method 
>> being used seemed inappropriate, even though it originally had the 
>> desired functionality.
>>
>> I'll make the new method final and update the doc comments. I 
>> generally defer updating copyright headers to minimize trivia in the 
>> webrev.
>>
>> -- Jon
>>
>> On 2/22/18 4:59 PM, Maurizio Cimadamore wrote:
>>> If I'm correct this fix doesn't really change anything in terms of 
>>> the implementation - the main goal is to give Symbol.isInheritedIn a 
>>> 'fresh' name so that the overridden version in MethodSymbol is never 
>>> called? Seems a tad subtle :-)
>>>
>>> I suggest making the new method 'final' to avoid possibilities of 
>>> similar issues in the future (e.g. if an override of 
>>> 'isAccessibleIn' is added, things will be broken again).
>>>
>>> Also, minor nit, the javadoc of the two sibling methods are slighty 
>>> off sync ("of the given class" vs. "of given class"); copyright 
>>> headers in tests could also use a refresh.
>>>
>>> Cheers
>>> Maurizio
>>>
>>>
>>> On 23/02/18 00:37, Jonathan Gibbons wrote:
>>>> Please review a simple fix to Symbol, to provide the ability to see 
>>>> if an element is accessible in a subtype, as distinct from being 
>>>> inherited.  This is needed by JavacElements.hides.
>>>>
>>>> The test was provided by Kumar, in the original bug report.
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8186688
>>>> Webrev: http://cr.openjdk.java.net/~jjg/8186688/webrev.00/
>>>>
>>>> -- Jon
>>>
>>
>



More information about the compiler-dev mailing list