JDK 9 request for specification changes of JDK-8032230: Enhance javax.a.p.RoundEnvironment after repeating annotations
joe darcy
joe.darcy at oracle.com
Thu May 26 02:46:10 UTC 2016
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