Skip to content

Conversation

@som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Nov 9, 2025

In canProfitFromMoreConstraints, balk if mode.is(ImplicitExploration).

The failing case was due to the constrainResult "up the stack" in adaptNoArgs, which drove type comparisons and completions, including a conversion. The FunProto context had a console reporter that emitted the errors encountered, not an exploring reporter.

The justification is that "more constraints" is motivated by arg inference, so if the arg is already involved in views, further information in the result is not needed, especially at the cost of spurious exploratory errors using a context that doesn't reflect the current mode.

Fixes #24192

@som-snytt som-snytt force-pushed the issue/24192-given-error branch from 5eb0d10 to 02b1caa Compare November 9, 2025 21:51
@som-snytt som-snytt marked this pull request as ready for review November 9, 2025 21:52
@Gedochao Gedochao requested a review from odersky November 18, 2025 11:13
Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely spotted! I have just one small change request.

case MethodType(ContextBoundParamName(_) :: _) => sourceVersion.isAtLeast(`3.4`)
case _ => false
*/
val needsUsing = true //wtp.isImplicitMethod is asserted at beginning of method
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that we should keep the second condition for needsUsing. We don't want to change behavior under -source 3.3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I guess the || was a boolean typo for &&.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my bad. At this point I don't know anymore what was intended. Never mind, we can keep it as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll spend a few minutes to think of a counter-example. More tests would be nice, and the details of the PR are already receding into the fogs of time.

@odersky odersky assigned som-snytt and unassigned odersky Nov 28, 2025
@som-snytt som-snytt force-pushed the issue/24192-given-error branch from 02b1caa to f3339f3 Compare November 28, 2025 19:51
@som-snytt som-snytt force-pushed the issue/24192-given-error branch from f3339f3 to 6b089ed Compare November 29, 2025 18:58
@som-snytt som-snytt force-pushed the issue/24192-given-error branch from 6b089ed to e351107 Compare November 29, 2025 20:54
@som-snytt
Copy link
Contributor Author

It now always takes needsUsing as true. Previously, the intention was just to avoid warning about requiring using for explicit context bounds; but the fix for "correctly show implicitNotFound message" relies on it as well. It's always OK to set the apply kind to using.

The latter fix is not needed when adaptNoArgs avoids the typecheck to elicit default args. A previous fix mostly does avoid it, except for the test case added to tests/neg/i19594.scala.

Maybe further tweakage is possible, but not pursued here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Given definition is no longer resolved to in mapped function usage site.

2 participants