RFR 8221118: Avoid eagerly creating JCDiagnostic for CompletionFailures

[Maurizio Cimadamore]

Looks good - for bonus points, you could condense the code a bit - e.g. 
turn this

Supplier<JCDiagnostic> diag =
+                () -> 
diagFactory.fragment(Fragments.ClassFileNotFound(c.flatname));
              return newCompletionFailure(c, diag);

into this:

              return newCompletionFailure(c, () -> 
diagFactory.fragment(Fragments.ClassFileNotFound(c.flatname)));


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...

I'll leave this to your judgement.

Maurizio

On 28/03/2019 22:42, forax at univ-mlv.fr wrote:
> Looks good to me.
>
> Rémi
>
> ------------------------------------------------------------------------
>
>     *De: *"Ron Shapiro" <ronshapiro at google.com>
>     *À: *"Remi Forax" <forax at univ-mlv.fr>
>     *Cc: *"jonathan gibbons" <jonathan.gibbons at oracle.com>,
>     "compiler-dev" <compiler-dev at openjdk.java.net>
>     *Envoyé: *Jeudi 28 Mars 2019 23:30:25
>     *Objet: *Re: RFR 8221118: Avoid eagerly creating JCDiagnostic for
>     CompletionFailures
>
>     Here's an updated webrev with resetDiagnostic():
>     http://cr.openjdk.java.net/~ronsh/8221118/webrev.04/
>
>     On Thu, Mar 28, 2019 at 6:14 PM <forax at univ-mlv.fr
>     <mailto:forax at univ-mlv.fr>> wrote:
>
>
>
>         ------------------------------------------------------------------------
>
>             *De: *"Ron Shapiro" <ronshapiro at google.com
>             <mailto:ronshapiro at google.com>>
>             *À: *"Remi Forax" <forax at univ-mlv.fr
>             <mailto:forax at univ-mlv.fr>>
>             *Cc: *"jonathan gibbons" <jonathan.gibbons at oracle.com
>             <mailto:jonathan.gibbons at oracle.com>>, "compiler-dev"
>             <compiler-dev at openjdk.java.net
>             <mailto:compiler-dev at openjdk.java.net>>
>             *Envoyé: *Jeudi 28 Mars 2019 22:58:00
>             *Objet: *Re: RFR 8221118: Avoid eagerly creating
>             JCDiagnostic for CompletionFailures
>
>             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?
>
>
>         resetDiagnostic() ?
>
>             Does this complicate the API of this type?
>
>
>         yes, but i believe the complexity comes from the fact you want
>         lazy loading thus it's not a plain data type anymore.
>
>         Rémi
>
>
>             On Thu, Mar 28, 2019 at 5:51 PM Remi Forax
>             <forax at univ-mlv.fr <mailto:forax at univ-mlv.fr>> wrote:
>
>                 Hi Ron,
>                 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.
>                 I also think that the supplier should not be public too.
>
>                 regards,
>                 Rémi
>
>                 ------------------------------------------------------------------------
>
>                     *De: *"Ron Shapiro" <ronshapiro at google.com
>                     <mailto:ronshapiro at google.com>>
>                     *À: *"jonathan gibbons"
>                     <jonathan.gibbons at oracle.com
>                     <mailto:jonathan.gibbons at oracle.com>>
>                     *Cc: *"compiler-dev"
>                     <compiler-dev at openjdk.java.net
>                     <mailto:compiler-dev at openjdk.java.net>>
>                     *Envoyé: *Mercredi 27 Mars 2019 18:46:09
>                     *Objet: *Re: RFR 8221118: Avoid eagerly creating
>                     JCDiagnostic for CompletionFailures
>
>                     I missed a few cases earlier - see this updated
>                     webrev:
>                     http://cr.openjdk.java.net/~ronsh/8221118/webrev.01/
>
>                     On Tue, Mar 26, 2019 at 4:10 PM Jonathan Gibbons
>                     <jonathan.gibbons at oracle.com
>                     <mailto:jonathan.gibbons at oracle.com>> wrote:
>
>                         Looks OK to me, especially the similarity to
>                         JDK-8215368
>
>                         -- Jon
>
>
>                         On 03/19/2019 03:37 PM, Ron Shapiro wrote:
>
>                             Hi,
>
>                             This is a small change that avoids eagerly
>                             creating JCDiagnostic for CompletionFailures.
>
>                             webrev:
>                             http://cr.openjdk.java.net/~ronsh/8221118/webrev.00/
>                             <http://cr.openjdk.java.net/%7Eronsh/8221118/webrev.00/>
>                             bug:
>                             https://bugs.openjdk.java.net/browse/JDK-8221118
>
>                             Thanks,
>                             Ron
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20190329/178d7b87/attachment-0001.html>