JDK 8 RFR, redux, for JDK-8005294 : Consider default methods for additions to AnnotatedElement

[Peter Levart]

On 10/28/2013 06:59 AM, Joe Darcy wrote:
> Hello Joel and Peter,
>
> Working in previous feedback, please review the revised code changes for
>
>     JDK-8005294 : Consider default methods for additions to 
> AnnotatedElement
>     http://cr.openjdk.java.net/~darcy/8005294.4/
>
> I think the versions of getAnnotationsByType and getDeclaredAnnotation 
> in this patch are solid, but getDeclaredAnnotationsByType may require 
> some more refinement. (The asserts in getDeclaredAnnotationsByType are 
> commented out since I was getting a bad build as a result; I'm 
> following up with the HotSpot team.)

Hi Joe,

After un-comenting asserts, the one in line 404...

              if (indirectlyPresent == null || indirectlyPresent.length 
== 0) {
                  ...
              } else {
                  //                 assert indirectlyPresent != null && 
indirectlyPresent.length > 0;

                  if (resultSize == indirectlyPresent.length){
404:                 //                     assert *resultSize == 0 ||* 
directlyPresent == null;


...should be changed to: "assert directlyPresent == null" since 
resultSize (== indirectlyPresent.length) is always > 0 in this line...


Then perhaps a small optimization in loop at line 416 to avoid calling 
annotationType() (a Proxy method) twice:

416:                 for (Annotation a : getDeclaredAnnotations()) {
*Class<? extends Annotation> aType = a.annotationType();*
                          if (*aType*.equals(annotationClass)) {
                              indirectOffset = 1;
                              break;
                          } else if (*aType*.equals(containerType)) {
                              break;
                          }
                      }


The test looks fine. I just wonder whether you intended to uncomment or 
to remove the following lines of AnnotatedElementDelegate:

  228 //     // FIXME -- adjust once build / vm problems resolved
  229 //     @Override
  230 //     public <T extends Annotation> T[] getDeclaredAnnotationsByType(Class<T> annotationClass) {
  231 //         return base.getDeclaredAnnotationsByType(annotationClass);
  232 //     }


...since by un-commenting you would defeat the purpose of testing the 
default method.


Maybe also add the following two annotations:

@AssociatedDirectOnSuperclassIndirectOnSubclass
@AssociatedIndirectOnSuperclassDirectOnSubclass



Regards, Peter

>
> Also new in this version, the test program fleshed out.
>
> Thank,
>
> -Joe