JDK 9 request for specification changes of JDK-8032230: Enhance javax.a.p.RoundEnvironment after repeating annotations
Jonathan Gibbons
jonathan.gibbons at oracle.com
Thu May 26 02:58:52 UTC 2016
Well, it did seem strange to have the overriding method (just) call the
method it was overriding ;-)
One alternative for your consideration:
Instead of
109 @Override
110 public Set<? extends Element> getElementsAnnotatedWithAny(TypeElement... a) {
111 // Default method defined in the interface
112 return RoundEnvironment.super.getElementsAnnotatedWithAny(a);
113 }
how about
109 // inherit default impl of this method from the interface
110 // public Set<? extends Element> getElementsAnnotatedWithAny(TypeElement... a)
Or, you could put comments on the class itself.
// This class delegates all abstract methods on the interface to
the {@code RoundEnvironment}
// provided when the class is created, relying on the default
methods provided in the interface
// for ....(fill in details)...
Whichever way you go, overriding the methods or not, I think some
clarifying comments are
in order.
-- Jon
On 05/25/2016 07:46 PM, joe darcy wrote:
> Hi Jon,
>
>
> On 5/25/2016 5:57 PM, Jonathan Gibbons wrote:
>> Joe,
>>
>> TestingRoundEnvironment is (mostly) cleaver, but why do you bother to
>> override the default methods, and then call them via super?
>>
>> Why not just not override them in the first place? Since this is a
>> class that is simply implementing an interface, the only method can
>> can be invoked is the default method from the interface.
>
> I thought, perhaps incorrectly, that explicitly calling the interface
> methods (as opposed to not overriding those methods) would more
> clearly indicate the intention of the wrapper class.
>
> Do you think a more explicit comment about this intention on the
> wrapper class would help clarify matters?
>
> -Joe
>
>>
>> -- Jon
>>
>>
>>
>> On 05/24/2016 09:18 AM, joe darcy wrote:
>>> Updated webrev:
>>>
>>> http://cr.openjdk.java.net/~darcy/8032230.1/
>>>
>>> Now with specialized implementation in javac's implementation of
>>> RoundEnvironment and separate tests of the default methods in the
>>> interface and the javac implementation.
>>>
>>> Thanks,
>>>
>>> -Joe
>>>
>>>
>>> On 5/23/2016 11:21 AM, joe darcy wrote:
>>>> Hello,
>>>>
>>>> Please now review a webrev including the specification and some tests:
>>>>
>>>> http://cr.openjdk.java.net/~darcy/8032230.0/
>>>>
>>>> Thanks,
>>>>
>>>> -Joe
>>>>
>>>>
>>>> On 5/18/2016 2:02 PM, joe darcy wrote:
>>>>> Combining suggestions, two methods
>>>>>
>>>>> default Set<? extends Element>
>>>>> getElementsAnnotatedWithAny(TypeElement... annotations)
>>>>>
>>>>> default Set<? extends Element>
>>>>> getElementsAnnotatedWithAny(Set<Class<? extends Annotation>>
>>>>> annotations)
>>>>>
>>>>> No varargs in the second case because of heap pollution with the
>>>>> wildcarded Class type and the inability to use @SafeVarargs on
>>>>> this method since it can be overridden.
>>>>>
>>>>> I'd prefer a set of methods like the above that weren't only tied
>>>>> to repeating annotations since I think there are other use cases
>>>>> where it is helpful to find more than one annotation at a time.
>>>>>
>>>>> Fuller diff below.
>>>>>
>>>>> Thanks,
>>>>>
>>>>> -Joe
>>>>>
>>>>>
>>>>> /**
>>>>> + * Returns the elements annotated with one or more of the given
>>>>> + * annotation types.
>>>>> + *
>>>>> + * @apiNote This method may be useful when processing repeating
>>>>> + * annotations by looking for an annotation type and its
>>>>> + * containing annotation type at the same time.
>>>>> + *
>>>>> + * @implSpec The default implementation of this method
>>>>> creates an
>>>>> + * empty result set, iterates over the annotations in the
>>>>> argument
>>>>> + * set calling {@link #getElementsAnnotatedWith(TypeElement)} on
>>>>> + * each annotation and adding those results to the result
>>>>> + * set. Finally, the contents of the result set are returned
>>>>> as an
>>>>> + * unmodifiable set.
>>>>> + *
>>>>> + * @param annotations annotation types being requested
>>>>> + * @return the elements annotated with one or more of the given
>>>>> + * annotation types, or an empty set if there are none
>>>>> + * @throws IllegalArgumentException if the any elements of the
>>>>> + * argument set do not represent an annotation type
>>>>> + * @since 9
>>>>> + */
>>>>> + default Set<? extends Element>
>>>>> getElementsAnnotatedWithAny(TypeElement... annotations){
>>>>> + HashSet<Element> result = new HashSet<>();
>>>>> + for (TypeElement annotation : annotations) {
>>>>> + result.addAll(getElementsAnnotatedWith(annotation));
>>>>> + }
>>>>> + return Collections.unmodifiableSet(result);
>>>>> + }
>>>>>
>>>>> + /**
>>>>> + * Returns the elements annotated with one or more of the given
>>>>> + * annotation types.
>>>>> + *
>>>>> + * @apiNote This method may be useful when processing repeating
>>>>> + * annotations by looking for an annotation type and its
>>>>> + * containing annotation type at the same time.
>>>>> + *
>>>>> + * @implSpec The default implementation of this method
>>>>> creates an
>>>>> + * empty result set, iterates over the annotations in the
>>>>> argument
>>>>> + * set calling {@link #getElementsAnnotatedWith(Class)} on
>>>>> + * each annotation and adding those results to the result
>>>>> + * set. Finally, the contents of the result set are returned
>>>>> as an
>>>>> + * unmodifiable set.
>>>>> + *
>>>>> + * @param annotations annotation types being requested
>>>>> + * @return the elements annotated with one or more of the given
>>>>> + * annotation types, or an empty set if there are none
>>>>> + * @throws IllegalArgumentException if the any elements of the
>>>>> + * argument set do not represent an annotation type
>>>>> + * @since 9
>>>>> + */
>>>>> + default Set<? extends Element>
>>>>> getElementsAnnotatedWithAny(Set<Class<? extends Annotation>>
>>>>> annotations){
>>>>> + HashSet<Element> result = new HashSet<>();
>>>>> + for (Class<? extends Annotation> annotation : annotations) {
>>>>> + result.addAll(getElementsAnnotatedWith(annotation));
>>>>> + }
>>>>> + return Collections.unmodifiableSet(result);
>>>>> + }
>>>>>
>>>>>
>>>>> On 5/18/2016 1:55 PM, Jonathan Gibbons wrote:
>>>>>> Jan,
>>>>>>
>>>>>> You're getting very close to suggesting a method that takes a
>>>>>> predicate, and we could provide a variety of predicates for
>>>>>> (anyOf, allOf) x (Annotation, AnnotationMirror) x (varags,
>>>>>> collection)
>>>>>>
>>>>>> -- Jon
>>>>>>
>>>>>> On 05/18/2016 01:48 PM, Jan Lahoda wrote:
>>>>>>> On 18.5.2016 22:26, joe darcy wrote:
>>>>>>>> Hi Jon,
>>>>>>>>
>>>>>>>> On 5/18/2016 1:05 PM, Jonathan Gibbons wrote:
>>>>>>>>> It's sad to see the preference given to the more intellectually
>>>>>>>>> suspect of the two possibilities.
>>>>>>>>
>>>>>>>> Agreed, sad but pragmatic.
>>>>>>>
>>>>>>> Would it make sense to use varargs instead of a Set? If we
>>>>>>> assume the typical use will be something like:
>>>>>>> round.getElementsAnnotatedWithAny(annotation, containerAnnotation)
>>>>>>>
>>>>>>> it might be more convenient to have the method take a vararg
>>>>>>> than a Set (and would avoid the problem with the erasure clash
>>>>>>> as another benefit).
>>>>>>>
>>>>>>> Jan
>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>> It would be nice to see the nice name given to the intellectually
>>>>>>>>> superior of the possibilities (i.e AnnotationMirror) and then
>>>>>>>>> figure
>>>>>>>>> out how to deal with the other case.
>>>>>>>>
>>>>>>>> How about the name "getElementsAnnotatedWithAny" for both
>>>>>>>> variations?
>>>>>>>> That potentially avoids confusion over whether or not the
>>>>>>>> elements have
>>>>>>>> to be modified with any of the annotations or all of them.
>>>>>>>>
>>>>>>>>>
>>>>>>>>> As well as the possibility of another method name, have you ever
>>>>>>>>> considered the possibility of conversion functions of
>>>>>>>>> Elements/Types/<something-new> that can convert between
>>>>>>>>> (collection
>>>>>>>>> of) Annotation and (collection of) AnnotationMirror?
>>>>>>>>
>>>>>>>> Internally, for the existing methods javac does convert the
>>>>>>>> Class-based
>>>>>>>> version to the TypeElement based version, but I don't think we
>>>>>>>> want the
>>>>>>>> specification to require that.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> -Joe
>>>>>>>>
>>>>>>>>>
>>>>>>>>> -- Jon
>>>>>>>>>
>>>>>>>>> On 05/18/2016 12:55 PM, joe darcy wrote:
>>>>>>>>>> Hello,
>>>>>>>>>>
>>>>>>>>>> Please review the patch below which proposes a new method to
>>>>>>>>>> address
>>>>>>>>>>
>>>>>>>>>> JDK-8032230: Enhance javax.a.p.RoundEnvironment after
>>>>>>>>>> repeating
>>>>>>>>>> annotations
>>>>>>>>>>
>>>>>>>>>> At present, this is just a review of the specification of the
>>>>>>>>>> default
>>>>>>>>>> method in the interface and *not* a more optimized
>>>>>>>>>> implementation in
>>>>>>>>>> the javac RoundEnvironemnt implementation (and not any tests
>>>>>>>>>> that
>>>>>>>>>> would be needed for the new functionality).
>>>>>>>>>>
>>>>>>>>>> Note that the bug proposes adding two methods
>>>>>>>>>>
>>>>>>>>>> RoundEnvironment.getElementsAnnotatedWith(Set<Class<? extends
>>>>>>>>>> Annotation>> s)
>>>>>>>>>> RoundEnvironment.getElementsAnnotatedWith(Set<AnnotationMirror>
>>>>>>>>>> s)
>>>>>>>>>>
>>>>>>>>>> but these methods would clash since their erasure is the
>>>>>>>>>> same. *sad
>>>>>>>>>> tromphone*
>>>>>>>>>>
>>>>>>>>>> Therefore, I'm only proposing to add the Class-based variant
>>>>>>>>>> since
>>>>>>>>>> that one is the more commonly used of the two.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> -Joe
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> + /**
>>>>>>>>>> + * Returns the elements annotated with any of the given
>>>>>>>>>> annotation
>>>>>>>>>> + * types.
>>>>>>>>>> + *
>>>>>>>>>> + * @apiNote This method may be useful when processing
>>>>>>>>>> repeating
>>>>>>>>>> + * annotations by looking for an annotation type and its
>>>>>>>>>> + * containing annotation type at the same time.
>>>>>>>>>> + *
>>>>>>>>>> + * @implSpec The default implementation of this method
>>>>>>>>>> creates an
>>>>>>>>>> + * empty result set, iterates over the annotations in
>>>>>>>>>> the argument
>>>>>>>>>> + * set calling {@link
>>>>>>>>>> #getElementsAnnotatedWith(TypeElement)} on
>>>>>>>>>> + * each annotation and adding those results to the result
>>>>>>>>>> + * set. Finally, the contents of the result set are
>>>>>>>>>> returned as an
>>>>>>>>>> + * unmodifiable set.
>>>>>>>>>> + *
>>>>>>>>>> + * @param annotations annotation type being requested
>>>>>>>>>> + * @return the elements annotated with the given
>>>>>>>>>> annotation types,
>>>>>>>>>> + * or an empty set if there are none
>>>>>>>>>> + * @throws IllegalArgumentException if the any elements
>>>>>>>>>> of the
>>>>>>>>>> + * argument set do not represent an annotation type
>>>>>>>>>> + * @since 9
>>>>>>>>>> + */
>>>>>>>>>> + default Set<? extends Element>
>>>>>>>>>> getElementsAnnotatedWith(Set<Class<? extends Annotation>>
>>>>>>>>>> annotations){
>>>>>>>>>> + HashSet<Element> result = new HashSet<>();
>>>>>>>>>> + for (Class<? extends Annotation> annotation :
>>>>>>>>>> annotations) {
>>>>>>>>>> + result.addAll(getElementsAnnotatedWith(annotation));
>>>>>>>>>> + }
>>>>>>>>>> + return Collections.unmodifiableSet(result);
>>>>>>>>>> + }
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the compiler-dev
mailing list