Consolidate / clean up flags relating to inference

eclipse-jdt / eclipse.jdt.core

Eclipse Public License 2.0

157 stars 124 forks source link

Consolidate / clean up flags relating to inference #2784

Open stephan-herrmann opened 1 month ago

stephan-herrmann commented 1 month ago

From observations in #2776:

Flag ParameterizedGenericMethodBinding.inferredReturnType is obsolete since 1.7 mode got disabled
- the same for significant portions of code in the computeCompatibleMethod family of methods
Flag ParameterizedGenericMethodBinding.wasInferred is used also in 1.8+ but it seems we might get a conflict with caching of these bindings.
- whether or not the flag is set to true depends on the call path, but not all call paths may be aware of this distinction

stephan-herrmann commented 1 month ago

@srikanth-sankaran , @iloveeclipse do you agree to holding back this clean-up to the next version?

@srikanth-sankaran @jjohnstn, the issue concerning wasInferred could perhaps cause wrong results from #2776 -> wrong refactoring in corner cases. At the moment this is only a theory. Still OK to defer to the next release?

iloveeclipse commented 1 month ago

do you agree to holding back this clean-up to the next version?

Why should we? It isn't clear for me what is proposed and which impact it can have. M3 is in two weeks from now, so if that is a bigger refactoring, better to wait for 4.34, if it is something smaller, why not for M3?

jjohnstn commented 1 month ago

the issue concerning wasInferred could perhaps cause wrong results from #2776 -> wrong refactoring in corner cases. At the moment this is only a theory. Still OK to defer to the next release?

I'm fine with a few corner cases if it needs to be deferred. They aren't likely to be triggered as user needs to explicitly do an inline.

stephan-herrmann commented 1 month ago

do you agree to holding back this clean-up to the next version?

Why should we? It isn't clear for me what is proposed

the change would amount roughly to discarding the 1.7 version of type inference. I guess that would mark a point of no return...

.. and which impact it can have. M3 is in two weeks from now, so if that is a bigger refactoring, better to wait for 4.34, if it is something smaller, why not for M3?

I think I should rather focus on BETA_JAVA23 at this point in time, except for any actual bugs perhaps. Beginning of new cycle sounds more like clean-up time. :)

stephan-herrmann commented 1 month ago

Flag ParameterizedGenericMethodBinding.wasInferred is used also in 1.8+ but it seems we might get a conflict with caching of these bindings.

Similar concern was already raised in https://bugs.eclipse.org/bugs/show_bug.cgi?id=174436#c5 - it seems the concern was not fully resolved back then? The API was surfaced on MethodInvocation (DOM) but it uses the ParameterizedGenericMethodBinding (compiler) for its computation.