webrev for type-annotations, repeating type-annotations test

[Steve Sides]

On 1/10/2013 2:37 PM, Alex Buckley wrote:
> Hi Steve,
>
> It seems to me there is still quite a lot of code duplication in 
> ClassfileTestHelper for the 3-arg and 4-arg versions of test().
But it's only in one file now! :)  It was previously in each test.
I stared at this for a while. I will try to factor it down a bit. It 
should be possible.

>
> The writeup of CombinationsTargetTest is helpful, especially the 
> sample output. It shows there is a lot going on: i) annotation types 
> which are declaration-site v. use-site v. decl-and-use-site, and ii) 
> annotations which occur once v. multiple times v. jointly with other 
> annotations. Two questions:
>
> 1. Could you expand "4) inside method bodies" to make clear what 
> locations are tested with type annotations?
Do you want I should add this to the comments or put it in comment in 
the test?
Each code scenario could have a comment blurb listing usages.

>
> 2. Is it possible to "limit" the test generation of 
> CombinationsTargetTest to just (say) use-site annotations occurring 
> once in cast expressions ? If a "simple" scenario like that is easy to 
> run and verify, it would give confidence that complex scenarios like 
> the sample output are correct.
I can, but that will have to follow a little later after this mail.

FWIW, I did find the bug for double annotations on new,array,cast using 
this test. It failed as noted in the webrevComment.txt file.
Once it fails, i can see there is a mismatch in the number of expected 
annotations, and I started narrowing it down...it wasn't that difficult.

This test case,  
http://cr.openjdk.java.net/~ssides/tests/TypeAnnoContainers.java, is 
essentially a smaller version of the whole thing.
The output of that is

METHOD: m1, RuntimeVisibleTypeAnnotations: 3
METHOD: m1, RuntimeInvisibleTypeAnnotations: 3
METHOD: m2, RuntimeVisibleTypeAnnotations: 3
METHOD: m2, RuntimeInvisibleTypeAnnotations: 3
all        : 12, expected :18
visibles   : 6, expected :9
invisibles : 6, expected :9

Exception in thread "main" java.lang.RuntimeException: Did not find 
expected type annotations.
         at TypeAnnoContainers.report(TypeAnnoContainers.java:56)
         at TypeAnnoContainers.run(TypeAnnoContainers.java:100)
         at TypeAnnoContainers.main(TypeAnnoContainers.java:46)

By looking at the code and the output above, you can easily identify 
which numbers are off.
There are some things which makes this easier to narrow down in that it 
shows the number of annotation per class,method,field(in this case method).

I have a utility, CountAnnotations.java, which came out of that, but I 
did not build that in to the larger test.  I could try to do that so you 
can get the above type of output for a test failure, but I'm not sure 
the test is structured so as to lend itself to that. FWIW, I posted that 
file at
http://cr.openjdk.java.net/~ssides/tests/CountAnnotations.java

-steve

>
> Alex
>
> On 1/9/2013 5:00 PM, Steve Sides wrote:
>> I posted a second webrev,
>> http://cr.openjdk.java.net/~ssides/8005085/webrev.02/
>> with a webrevComment.txt file and removed the statics. :)
>>
>> -steve
>>
>> On 1/9/2013 12:32 PM, Alex Buckley wrote:
>>> Hi Steve,
>>>
>>> Thanks for publishing this webrev. (And congrats on becoming an Author
>>> in this project!)
>>>
>>> CombinationsTargetTest is pretty complicated. Could you please give a
>>> description and examples of the combinations it generates?
>>>
>>> DeadCode, NewTypeArguments, TypeCasts, and Wildcards all use static
>>> fields in their superclass (ClassfileTestHelper) to communicate
>>> expected results to ClassfileTestHelper.countAnnotations(). Is it
>>> necessary to use static fields for this purpose?
>>>
>>> 8005681 is interesting. You're saying that "new @Foo @Foo @Bar @Bar
>>> C()" causes all four annotations to be dropped? Werner cannot access
>>> JBS, so it would be helpful if you could send test cases to this list.
>>>
>>> Alex
>>>
>>> On 1/8/2013 3:45 PM, Steve Sides wrote:
>>>> This test supercedes the test in the previous review request (which 
>>>> was
>>>> via mail).
>>>> http://cr.openjdk.java.net/~ssides/8005085/
>>>>
>>>> In writing a test for repeating type annotations I found it could 
>>>> cover
>>>> several scenarios in one test, so this coverage
>>>> mixed target types on type usages and type parameters as well as
>>>> repeating type annotations in those usages.
>>>>
>>>> Due to http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005681 a 
>>>> few
>>>> scenarios are commented out.
>>>> In addition to any other changes or suggestions, we may want for 
>>>> that to
>>>> be fixed so the whole test can be considered.
>>>>
>>>> This also include the previous refactoring of the classfile tests 
>>>> into a
>>>> helper, ClassfileTestHelper.java.
>>>> This makes the tests much simpler looking and easier to add new tests.
>>>>
>>>> I'd appreciate any comments or suggestions.
>>>>
>>>> -steve
>>