RFR 6388543: improve accuracy of source positions for AnnotationValue param of Messager.printMessage

[Jonathan Gibbons]

Hi Liam,

I have reviewed the spec for the method, the existing code, the bug 
report and its comments, and your notes in this email thread.

I note that reference equality is being used, and I agree it seems to 
give the best semantics, as regards avoiding ambiguities. I also note 
that the search for the annotation mirror is recursive. Although not 
functionally necessary, this makes me wonder whether we should not also 
search recursively for the annotation value. That would seem to give the 
best flexibility to the user, allowing them to find a match for @A + 2, 
to use the example in this thread.

Is there any good reason not to do so?

-- Jon


On 01/12/2017 11:23 AM, Liam Miller-Cushon wrote:
> Any more thoughts on this? I'm happy to revise the approach if there's 
> a better way to use the hint.
>
> On Tue, Jan 3, 2017 at 10:28 PM, Liam Miller-Cushon <cushon at google.com 
> <mailto:cushon at google.com>> wrote:
>
>     Thanks for the feedback,
>
>     On Tue, Jan 3, 2017 at 9:00 PM, Jonathan Gibbons
>     <jonathan.gibbons at oracle.com <mailto:jonathan.gibbons at oracle.com>>
>     wrote:
>
>         I guess I am slightly surprised by the proposed solution, as
>         regards the choice of what it means to find a match. I have
>         always assumed that any implementation would iterate over the
>         element values of the annotation, looking for a value that is
>         .equals to the given value, for some appropriate definition of
>         AnnotationValue.equals. It does not seem obvious to me that
>         you should be searching within the individual element values
>         for the match. I am also surprised that you might match for @B
>         + 2; I would expect the annotation mirror to only match at the
>         top level.   For example, assuming a "first one wins" policy,
>         I would have expected @B + 2 to match the second annotation in
>         the pair of annotations `@A(x = {1}, y = @B(2)) @B(2)`
>
>
>     I thought the proposed approach was consistent with the existing
>     handling of AnnotationMirror. The overloads of matchAnnoToTree use
>     reference equality to find a matching AnnotationMirror, which
>     could be either a top-level or nested annotation on the element.
>     So given `@A(x = {1}, y = @B(2)) @B(2)`, passing an
>     AnnotationMirror for @B to printMessage(..., Element,
>     AnnotationMirror) could match either appearance of @B depending on
>     which instance the argument was.
>
>     Is there a definition of AnnotationValue.equals that would be more
>     appropriate than reference equality? Value equality would be
>     ambiguous for e.g. `@A(x=1, y=1, z=1)` and @A + 1.
>
>     The motivation for supporting array elements was the same one
>     mentioned in the bug [1], which I've heard repeated elsewhere: for
>     annotations containing large array initializers it'd be helpful to
>     place the caret at a specific element instead of the start of the
>     array.
>
>     Liam
>
>     [1]
>     https://bugs.openjdk.java.net/browse/JDK-6388543?focusedCommentId=12267020&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-12267020
>     <https://bugs.openjdk.java.net/browse/JDK-6388543?focusedCommentId=12267020&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-12267020>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20170112/ad9976c1/attachment.html>