Skip to content

Fix issue with pc breaking in requiredMethod on newly overloaded valueOf #23708

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: main
Choose a base branch
from

Conversation

jchyb
Copy link
Contributor

@jchyb jchyb commented Aug 11, 2025

We recently started having issues when running completions in metals, with the presentation compiler crashing:

dotty.tools.dotc.core.TypeError$.apply(TypeErrors.scala:54)
	dotty.tools.dotc.core.Denotations$MultiDenotation.suchThat(Denotations.scala:1280)
	dotty.tools.dotc.core.Denotations$Denotation.requiredSymbol(Denotations.scala:305)
	dotty.tools.dotc.core.Denotations$Denotation.requiredMethod(Denotations.scala:321)
	dotty.tools.pc.completions.Completions.isUninterestingSymbol$lzyINIT1(Completions.scala:744)
	dotty.tools.pc.completions.Completions.isUninterestingSymbol(Completions.scala:725)
	dotty.tools.pc.completions.Completions.includeSymbol(Completions.scala:90)
	dotty.tools.pc.completions.Completions.visit$3(Completions.scala:697)
	dotty.tools.pc.completions.Completions.filterInteresting$$anonfun$1(Completions.scala:715)
	scala.collection.immutable.List.foreach(List.scala:334)
	dotty.tools.pc.completions.Completions.filterInteresting(Completions.scala:715)
	dotty.tools.pc.completions.Completions.enrichedCompilerCompletions(Completions.scala:118)
	dotty.tools.pc.completions.Completions.completions(Completions.scala:136)
	dotty.tools.pc.completions.CompletionProvider.completions(CompletionProvider.scala:139)
	dotty.tools.pc.ScalaPresentationCompiler.complete$$anonfun$1(ScalaPresentationCompiler.scala:194)


dotty.tools.dotc.core.TypeError$$anon$1: Failure to disambiguate overloaded reference with
  method valueOf in object Predef: [T]: T  and
  method valueOf in object Predef: [T](implicit vt: ValueOf[T]): T

This happened after the recent changes to the stdlib, with the, I believe, newly added valueOf overload, and the previously used requiredMethod by design not handling these cases.

I also noticed that in the same piece of code we had a bit of an empty Symbol factory situation going on, with MultiDenotation being changed into a Symbol (always resulting in an empty symbol), and only later flattened with the alternatives, so I changed that too.

I can't really test this properly, as the pc tests seem to use an older stdlib, but at least the wait methods do resolve properly after these changes, so I have no reason to think the valueOf methods would be any different.

@jchyb jchyb added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Aug 11, 2025
@jchyb jchyb added this to the 3.7.3 milestone Aug 11, 2025
// NOTE(olafur) IntelliJ does not complete the root package and without this filter
// then `_root_` would appear as a completion result in the code `foobar(_<COMPLETE>)`
defn.RootPackage,
// NOTE(gabro) valueOf was added as a Predef member in 2.13. We filter it out since is a niche
// use case and it would appear upon typing 'val'
defn.ValueOfClass.info.member(nme.valueOf).symbol,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ValueOf.valueOf does not exist and going by the comment, it seems that the ValueOf class was meant to be filtered here instead, so I changed this.

Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@tgodzik tgodzik enabled auto-merge (squash) August 11, 2025 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants