<div dir="ltr"><div dir="ltr">Sure thing. Here's that change:Â <a href="http://cr.openjdk.java.net/~ronsh/8221118/webrev.05/">http://cr.openjdk.java.net/~ronsh/8221118/webrev.05/</a></div><div dir="ltr"><br></div><div>Thanks for the review!</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Mar 29, 2019 at 7:40 AM Maurizio Cimadamore <<a href="mailto:maurizio.cimadamore@oracle.com">maurizio.cimadamore@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<p>Looks good - for bonus points, you could condense the code a bit
- e.g. turn this</p>
<p>Supplier<JCDiagnostic> diag =<br>
+Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â () ->
diagFactory.fragment(Fragments.ClassFileNotFound(c.flatname));<br>
            return newCompletionFailure(c, diag);<br>
<br>
</p>
<p>into this:</p>
<p>Â Â Â Â Â Â Â Â Â Â Â Â return newCompletionFailure(c, () ->
diagFactory.fragment(Fragments.ClassFileNotFound(c.flatname)));<br>
<br>
<br>
When the local var type used to be JCDiagnostic it was sort of ok,
now we have all these Supplier<JCDiagnostic> plus a
throwaway variable name...</p>
<p>I'll leave this to your judgement.<br>
</p>
<p>Maurizio<br>
</p>
On 28/03/2019 22:42, <a class="gmail-m_-5630617797497365935moz-txt-link-abbreviated" href="mailto:forax@univ-mlv.fr" target="_blank">forax@univ-mlv.fr</a> wrote:<br>
<blockquote type="cite">
<div style="font-family:arial,helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<div>Looks good to me.<br>
</div>
<div><br>
</div>
<div>Rémi<br>
</div>
<div><br>
</div>
<hr id="gmail-m_-5630617797497365935zwchr">
<div>
<blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><b>De:
</b>"Ron Shapiro" <a class="gmail-m_-5630617797497365935moz-txt-link-rfc2396E" href="mailto:ronshapiro@google.com" target="_blank"><ronshapiro@google.com></a><br>
<b>À: </b>"Remi Forax" <a class="gmail-m_-5630617797497365935moz-txt-link-rfc2396E" href="mailto:forax@univ-mlv.fr" target="_blank"><forax@univ-mlv.fr></a><br>
<b>Cc: </b>"jonathan gibbons"
<a class="gmail-m_-5630617797497365935moz-txt-link-rfc2396E" href="mailto:jonathan.gibbons@oracle.com" target="_blank"><jonathan.gibbons@oracle.com></a>, "compiler-dev"
<a class="gmail-m_-5630617797497365935moz-txt-link-rfc2396E" href="mailto:compiler-dev@openjdk.java.net" target="_blank"><compiler-dev@openjdk.java.net></a><br>
<b>Envoyé: </b>Jeudi 28 Mars 2019 23:30:25<br>
<b>Objet: </b>Re: RFR 8221118: Avoid eagerly creating
JCDiagnostic for CompletionFailures<br>
</blockquote>
</div>
<div>
<blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt">
<div dir="ltr">
<div dir="ltr">Here's an updated webrev with
resetDiagnostic():Â <a href="http://cr.openjdk.java.net/~ronsh/8221118/webrev.04/" target="_blank">http://cr.openjdk.java.net/~ronsh/8221118/webrev.04/</a><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Mar 28, 2019 at
6:14 PM <<a href="mailto:forax@univ-mlv.fr" target="_blank">forax@univ-mlv.fr</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<div style="font-family:arial,helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)"><br>
<br>
<hr id="gmail-m_-5630617797497365935gmail-m_-6406911898950855197zwchr">
<div>
<blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><b>De:
</b>"Ron Shapiro" <<a href="mailto:ronshapiro@google.com" target="_blank">ronshapiro@google.com</a>><br>
<b>À: </b>"Remi Forax" <<a href="mailto:forax@univ-mlv.fr" target="_blank">forax@univ-mlv.fr</a>><br>
<b>Cc: </b>"jonathan gibbons" <<a href="mailto:jonathan.gibbons@oracle.com" target="_blank">jonathan.gibbons@oracle.com</a>>,
"compiler-dev" <<a href="mailto:compiler-dev@openjdk.java.net" target="_blank">compiler-dev@openjdk.java.net</a>><br>
<b>Envoyé: </b>Jeudi 28 Mars 2019 22:58:00<br>
<b>Objet: </b>Re: RFR 8221118: Avoid eagerly
creating JCDiagnostic for CompletionFailures<br>
</blockquote>
</div>
<div>
<blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt">
<div dir="ltr">It's currently being reset by the
cached CompletionFailure in ClassFinder - I
can replace that with a method call that
resets the supplier and then nulls the
diagnostic. Suggestions on a method name for
this? </div>
</blockquote>
<br>
<div>resetDiagnostic() ?<br>
</div>
<br>
<blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt">
<div dir="ltr">Does this complicate the API of
this type?</div>
</blockquote>
<br>
<div>yes, but i believe the complexity comes from
the fact you want lazy loading thus it's not a
plain data type anymore.<br>
</div>
<br>
<div>Rémi<br>
</div>
<br>
<blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Mar
28, 2019 at 5:51 PM Remi Forax <<a href="mailto:forax@univ-mlv.fr" target="_blank">forax@univ-mlv.fr</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<div style="font-family:arial,helvetica,sans-serif;font-size:12pt;color:rgb(0,0,0)">
<div>Hi Ron,<br>
</div>
<div>In Symbol.java, having the field
'diag' being public means that you can
access to the object without calling
the getter so without calling the
supplier.<br>
</div>
<div>I also think that the supplier
should not be public too.<br>
</div>
<br>
<div>regards,<br>
</div>
<div>Rémi<br>
</div>
<br>
<hr id="gmail-m_-5630617797497365935gmail-m_-6406911898950855197gmail-m_-5622536266332603815zwchr">
<div>
<blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt"><b>De:
</b>"Ron Shapiro" <<a href="mailto:ronshapiro@google.com" target="_blank">ronshapiro@google.com</a>><br>
<b>À: </b>"jonathan gibbons" <<a href="mailto:jonathan.gibbons@oracle.com" target="_blank">jonathan.gibbons@oracle.com</a>><br>
<b>Cc: </b>"compiler-dev" <<a href="mailto:compiler-dev@openjdk.java.net" target="_blank">compiler-dev@openjdk.java.net</a>><br>
<b>Envoyé: </b>Mercredi 27 Mars
2019 18:46:09<br>
<b>Objet: </b>Re: RFR 8221118:
Avoid eagerly creating JCDiagnostic
for CompletionFailures<br>
</blockquote>
</div>
<div>
<blockquote style="border-left:2px solid rgb(16,16,255);margin-left:5px;padding-left:5px;color:rgb(0,0,0);font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt">
<div dir="ltr">
<div dir="ltr">I missed a few
cases earlier - see this updated
webrev: <a href="http://cr.openjdk.java.net/~ronsh/8221118/webrev.01/" target="_blank">http://cr.openjdk.java.net/~ronsh/8221118/webrev.01/</a><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On
Tue, Mar 26, 2019 at 4:10 PM
Jonathan Gibbons <<a href="mailto:jonathan.gibbons@oracle.com" target="_blank">jonathan.gibbons@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<p>Looks OK to me, especially
the similarity to
JDK-8215368</p>
<p>-- Jon<br>
</p>
<br>
<div class="gmail-m_-5630617797497365935gmail-m_-6406911898950855197gmail-m_-5622536266332603815gmail-m_-5081185188156804245moz-cite-prefix">On
03/19/2019 03:37 PM, Ron
Shapiro wrote:<br>
</div>
<blockquote>
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">
<div dir="ltr">Hi,<br>
<div><br>
</div>
<div>This is a small
change that avoids
eagerly creating
JCDiagnostic for
CompletionFailures.</div>
<div><br>
</div>
<div>webrev:Â <a href="http://cr.openjdk.java.net/%7Eronsh/8221118/webrev.00/" target="_blank">http://cr.openjdk.java.net/~ronsh/8221118/webrev.00/</a><br>
</div>
<div>bug:Â <a href="https://bugs.openjdk.java.net/browse/JDK-8221118" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8221118</a><br>
</div>
<div><br>
</div>
<div>Thanks,</div>
<div>Ron</div>
</div>
</div>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</blockquote>
</div>
</div>
</blockquote>
</div>
</blockquote></div>