Content-Length: 296788 | pFad | http://github.com/scala/scala/pull/11052

BE Judicious shadowing of implicits from scope by som-snytt · Pull Request #11052 · scala/scala · GitHub
Skip to content

Judicious shadowing of implicits from scope #11052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: 2.13.x
Choose a base branch
from

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented May 6, 2025

Fixes scala/bug#12351
Fixes scala/bug#9208

Retire both the old and new "implicit shadowing" mechanisms in favor of using ctx.lookupSymbol.

Tweak when ctx.implicitss collects implicits from a class by waiting to use owner.thisType.implicitMembers.

A notable improvement in correctness is that overloaded implicits are handled correctly (see neg/t729.scala). The spec for lexically scoped implicits is that the value must be "accessible without a prefix". (Probably a member was seen once in a nested context and then again as an overload in the class type, which was taken as shadowed.)

The shadowing test in isQualifyingImplicit is removed; further clean-up of removed code is still needed after more testing.

There are two test tweaks to address which are due to existing cyclic error handling: in one case (t712), where one implicit member causes a cycle, the other implicit member is not collected; in the other case (virtpatmat_typetag), an explicit type is spuriously required for a class tag, also due to an existing cyclic error.

Also 2509 switched order when reporting ambiguous, which is supposed to have "winner" first. Maybe benign or progress.

@scala-jenkins scala-jenkins added this to the 2.13.17 milestone May 6, 2025
@lrytz
Copy link
Member

lrytz commented May 6, 2025

sounds good! i'll need to find some time to look into it. want to run a community build on it early?

@som-snytt
Copy link
Contributor Author

som-snytt commented May 6, 2025

@lrytz thanks, before late bedtime I almost gave you a heads up to ask if you'd like me to continue the PR.

Before spending electrons on CB, I'll try some projects locally. I expect to discover more behaviors.

Edit: the cycle will need fixing first, I hit it right away. I'll leave the PR open out of sheer optimism.

@som-snytt
Copy link
Contributor Author

som-snytt commented May 11, 2025

Maybe there's nothing better to do than catch & ignore.

This is current behavior, regular type check fails in "assign type to tree" because asking if it's a refinement (typedDefDef) means checking all overrides. I see dotty has a <refinement> artifact. My bigger idea was that lookupSymbol need not force in lookupInPrefix, since it just needs to search parents for the name to detect that there is a symbol with that name; actually getting the (possibly overloaded) symbol is not necessary when probing. typedIdent tries a few strategies; I hoped gentler probing might help with cycles, but maybe wishful thinking.

[log typer] collect local implicits List(primary constructor $anon, type T, getter tTag, value tTag )
[log typer] collect local implicits List(<$anon: <empty>.this.Extractors>)
[log typer] collect local implicits List(type skolem S&0, value a, value evidence$1)
[log typer] collect local implicits List()
[log typer] collect local implicits List(primary constructor C, method extractorMatch)
huh.scala:20: tTag is not a valid implicit value for reflect.this.ClassTag[$anon.this.T] because:
illegal cyclic reference involving getter tTag
    (new Extractors { type T = S; val tTag = classTag[T] })(a)
                                                     ^

I haven't looked at what problem is solved by "pre-filtering". typedImplicit already says we're checking shadowing to reduce ambiguity, and typedIdent is basically just the lookup. It's nice to think that filtering is efficient, but not at the cost of correctness; and maybe it had more benefit in ancient days.

@som-snytt som-snytt force-pushed the issue/12351-implicit-in-self-type branch from 2b25baf to 111db9d Compare May 11, 2025 18:42
@som-snytt som-snytt force-pushed the issue/12351-implicit-in-self-type branch from 111db9d to 23a43f2 Compare May 11, 2025 18:50
@som-snytt som-snytt marked this pull request as ready for review May 13, 2025 18:58
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.

Type inference affects on implicit parameters inside traits? Implicit lookup interacts with import order
3 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/scala/scala/pull/11052

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy