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 00:57:35 UTC 2016
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.
-- 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