[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-compiler-dev
Subject: Re: RFR 6388543: improve accuracy of source positions for AnnotationValue param of Messager.printMes
From: Jonathan Gibbons <jonathan.gibbons () oracle ! com>
Date: 2017-02-07 2:05:01
Message-ID: 58992B4D.3010509 () oracle ! com
[Download RAW message or body]
reviewed, built, tested and pushed
-- Jon
On 02/06/2017 05:51 PM, Liam Miller-Cushon wrote:
> Thanks for the review! I fixed the nits, here's the latest version:
>
> http://cr.openjdk.java.net/~cushon/6388543/webrev.02/
> <http://cr.openjdk.java.net/%7Ecushon/6388543/webrev.02/>
>
> The changeset is attached.
>
> On Mon, Feb 6, 2017 at 3:40 PM, Jonathan Gibbons
> <jonathan.gibbons@oracle.com <mailto:jonathan.gibbons@oracle.com>> wrote:
>
>
>
> On 02/06/2017 10:45 AM, Liam Miller-Cushon wrote:
> > Hi, have you had a chance to look at the latest version of the
> > patch?
> >
> > And would it make sense to defer to 10?
> >
> > On Sat, Jan 14, 2017 at 3:37 PM, Liam Miller-Cushon
> > <cushon@google.com <mailto:cushon@google.com>> wrote:
> >
> > On Thu, Jan 12, 2017 at 1:46 PM, Jonathan Gibbons
> > <jonathan.gibbons@oracle.com
> > <mailto:jonathan.gibbons@oracle.com>> wrote:
> >
> > Is there any good reason not to do so?
> >
> >
> > I don't think so, thanks. I had thought that it would add
> > implementation complexity and wasn't as obviously useful as
> > supporting top-level and array elements values. However I
> > realized that matchAnnoToTree can be generalized to search
> > recursively for values, so it ends up being simpler. And as
> > you point out, it offers the best flexibility.
> >
> > I have uploaded a new version of the patch that searches
> > recursively:
> > http://cr.openjdk.java.net/~cushon/6388543/webrev.01/
> > <http://cr.openjdk.java.net/%7Ecushon/6388543/webrev.01/>
> >
> >
>
> http://cr.openjdk.java.net/~cushon/6388543/webrev.01/test/tools/javac/processing/messager/6388543/T6388543.java.html
> <http://cr.openjdk.java.net/%7Ecushon/6388543/webrev.01/test/tools/javac/processing/messager/6388543/T6388543.java.html>
>
> Nit: in the @modules, you don't need to specify java.compiler as
> well as jdk.compiler, since the latter requires the former.
> Nit: it is not common to see javax.* imports sorted before java.*
> imports.
>
> Otherwise, looks OK.
>
> I think this is small and safe enough for 9.
>
> -- Jon
>
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
reviewed, built, tested and pushed<br>
<br>
-- Jon<br>
<br>
<div class="moz-cite-prefix">On 02/06/2017 05:51 PM, Liam
Miller-Cushon wrote:<br>
</div>
<blockquote
cite="mid:CAL4Qsgu24BzVmgYmuBf3=Y6tVz3OPxFN5Brd2qNAVOKOz=zRpQ@mail.gmail.com"
type="cite">
<div dir="ltr">Thanks for the review! I fixed the nits, here's the
latest version:
<div><br>
</div>
<div><a moz-do-not-send="true"
href="http://cr.openjdk.java.net/%7Ecushon/6388543/webrev.02/">http://cr.openjdk.java.net/~cushon/6388543/webrev.02/</a></div>
<div><br>
</div>
<div>The changeset is attached.</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Mon, Feb 6, 2017 at 3:40 PM,
Jonathan Gibbons <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:jonathan.gibbons@oracle.com" \
target="_blank">jonathan.gibbons@oracle.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span class=""> <br>
<br>
<div class="m_-8357567548030413821moz-cite-prefix">On
02/06/2017 10:45 AM, Liam Miller-Cushon wrote:<br>
</div>
</span>
<div>
<div class="h5">
<blockquote type="cite">
<div dir="ltr">Hi, have you had a chance to look at
the latest version of the patch?
<div><br>
</div>
<div>And would it make sense to defer to 10?</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Sat, Jan 14, 2017 at
3:37 PM, Liam Miller-Cushon <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:cushon@google.com"
target="_blank">cushon@google.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0
0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_extra"><span>
<div class="gmail_quote">On Thu, Jan 12,
2017 at 1:46 PM, Jonathan Gibbons <span
dir="ltr"><<a
moz-do-not-send="true"
href="mailto:jonathan.gibbons@oracle.com"
\
target="_blank">jonathan.gibbons@oracle.com</a>></span> wrote:<br>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">Is
there any good reason not to do so?</blockquote>
</div>
<br>
</span>I don't think so, thanks. I had
thought that it would add implementation
complexity and wasn't as obviously useful
as supporting top-level and array elements
values. However I realized that
matchAnnoToTree can be generalized to
search recursively for values, so it ends
up being simpler. And as you point out, it
offers the best flexibility.</div>
<div class="gmail_extra"><br>
</div>
<div class="gmail_extra">I have uploaded a
new version of the patch that searches
recursively:</div>
<div class="gmail_extra"><a
moz-do-not-send="true"
\
href="http://cr.openjdk.java.net/%7Ecushon/6388543/webrev.01/"
\
target="_blank">http://cr.openjdk.java.net/~cu<wbr>shon/6388543/webrev.01/</a></div> \
</div> </blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</div>
</div>
<a moz-do-not-send="true"
class="m_-8357567548030413821moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ecushon/6388543/webrev.01/test/tools/javac/processing/messager/6388543/T6388543.java.html"
target="_blank">http://cr.openjdk.java.net/~<wbr>cushon/6388543/webrev.01/test/<wbr>tools/javac/processing/<wbr>messager/6388543/T6388543.<wbr>java.html</a><br>
Nit: in the @modules, you don't need to specify
java.compiler as well as jdk.compiler, since the latter
requires the former.<br>
Nit: it is not common to see javax.* imports sorted before
java.* imports.<br>
<br>
Otherwise, looks OK.<br>
<br>
I think this is small and safe enough for 9.<br>
<br>
-- Jon<br>
<br>
</div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic