RFR: 8325859: potential information loss during type inference [v3]

[Vicente Romero]

On Mon, 12 May 2025 15:40:12 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> > > The way this was designed, IIRC, was that if MIC is derived from IC, every time a variable is inferred in MIC, the resolution is forwarded to all the equivalent type variables in IC. If there's more type-variables in IC that are neither contained in MIC, nor equivalent to another var contained in MIC, it seems like an issue, as the minimization algo is not designed to work in this way.
> > 
> > 
> > yes I think this is an issue of the minimization algo. I mean there will always be at least one variable in MIC that won't be in IC and if MIC is asked to substitute that variable by its corresponding inference variable, then MIC won't be able to do it as it doesn't know how to map it and, potentially, some constraints won't be added to the inference variable. When we obtain MIC from IC we just don't know what queries we will do on MIC. The algo doesn't have this info.
> > I guess that there could be two ways to fix this:
> > ```
> > * keeping links to the original inference contexts which will have more info, which is what this PR is doing
> > 
> > * trying to generate a more complete MIC gathering additional info
> > ```
> > 
> > 
> >     
> >       
> >     
> > 
> >       
> >     
> > 
> >     
> >   
> > the second option seems like more complex as we could need some heuristics like type variables appearing in the same type, etc, and it could be harder to make sure that we get all the needed info. Also a more fluffy MIC would impact performance
> 
> I agree that keeping a link to the original context is conceptually simpler -- I guess I'm worried that the implementation started off one way (e.g. avoid a link between context, and use free type listeners) and now it's moving in a different way (adding links between context). Which means that now we have both kinds of complexities.
> 
> More concretely, `InferenceContext::min` has code to track dependencies via listeners. This PR doesn't remove that code, but it adds a _new_ field, by which an inference context could point to its original version. Then it adds some more code to make sure that when we instantiate a type var on a context, all the contexts it depend on get the memo. While I believe this fixes the issue, I wouldn't be surprised if the original inference context would now be notified multiple times, once through the legacy mechanism, and once through the new mechanism.
> 
> Is it possible to simplify the code so that the old mechanism goes away, and we do everything using the new field?

I have made several experiments to avoid the use of the free type listeners, the prototypes are just moving the code in the listeners to other places basically keeping the same level of complexity but trying to remove the timing component associated to the listeners. I have fixed several bugs or missing parts in the prototypes but the build then fails somewhere else. Also above you said:

>Then it adds some more code to make sure that when we instantiate a type var on a context, all the contexts it depend on get the memo

there is no cascade notification to other contexts when a type var is instantiated, the new code is just using the supplementary contexts as a kind of dictionary to look for the inference vars a given inference context doesn't know about. Given that there are no cascade notifications I don't think that the original inference context will be notified multiple times. Anyways multiple notification should not be possible as every time a free type listener is executed it is removed from the free type listener's map.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/25011#issuecomment-2881355329