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

Merged
merged 3 commits into from
Aug 12, 2025

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
@jchyb jchyb force-pushed the fix-pc-valueof-error branch from bae0924 to a4d07aa Compare August 12, 2025 12:34
@tgodzik tgodzik merged commit 1b5e93b into scala:main Aug 12, 2025
45 checks passed
WojciechMazur pushed a commit that referenced this pull request Aug 12, 2025
…eOf (#23708)

We recently started having issues when running completions in metals,
with the presentation compiler crashing:
```scala
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.
[Cherry-picked 1b5e93b]
WojciechMazur added a commit that referenced this pull request Aug 13, 2025
…oaded valueOf" to 3.7.3 (#23724)

Backports #23708 to the 3.7.3-RC2.

PR submitted by the release tooling.
[skip ci]
@WojciechMazur WojciechMazur added backport:done This PR was successfully backported. and removed backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. labels Aug 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:done This PR was successfully backported.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants