Skip to content
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

Attempt to fix completion crash #16267

Merged
merged 1 commit into from
Dec 1, 2022
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Nov 1, 2022

Fixes #16228 (hopefully)

Note: I could not test this. Maybe somebody else take can this over.

Looking at the stack trace in #16228:

at dotty.tools.dotc.core.TypeErasure.dotty$tools$dotc$core$TypeErasure$$sigName(TypeErasure.scala:815)
	at dotty.tools.dotc.core.TypeErasure$.sigName(TypeErasure.scala:204)
	at dotty.tools.dotc.core.Signature.$anonfun$2(Signature.scala:111)
	at scala.collection.immutable.List.map(List.scala:246)
	at dotty.tools.dotc.core.Signature.prependTermParams(Signature.scala:111)
	at dotty.tools.dotc.core.Types$MethodOrPoly.computeSignature$2(Types.scala:3565)
	at dotty.tools.dotc.core.Types$MethodOrPoly.signature(Types.scala:3582)
	at dotty.tools.dotc.core.Types$MethodOrPoly.computeSignature$2(Types.scala:3557)
	at dotty.tools.dotc.core.Types$MethodOrPoly.signature(Types.scala:3582)
	at dotty.tools.dotc.core.Denotations$SingleDenotation.signature(Denotations.scala:615)
	at dotty.tools.dotc.core.Denotations$SingleDenotation.signature(Denotations.scala:605)
	at dotty.tools.dotc.core.Denotations$SingleDenotation.atSignature(Denotations.scala:647)
	at dotty.tools.dotc.core.Denotations$SingleDenotation.atSignature(Denotations.scala:645)
	at dotty.tools.dotc.core.tasty.TreeUnpickler$TreeReader.readLengthTerm$1(TreeUnpickler.scala:1299)

I think what happens is that we need to disambiguate an overloaded reference by looking at the signature of each overloaded alternative. But one of the signatures refers to a type that's not on the classpath, which causes the MissingType exception. The fix is to catch the exception and declare the alternative that caused it to be non-matching.

About the regression: I have no idea what previous fix could have caused this.

@prolativ
Copy link
Contributor

prolativ commented Nov 2, 2022

This change seems to prevent the crash but at the same time this introduces a further regression - up until now the completion worked if a user added some prefix before triggering code completion given that this excluded the missing class from the completion suggestions. I'll see if I manage to somehow bisect the regression

@odersky
Copy link
Contributor Author

odersky commented Nov 18, 2022

This change seems to prevent the crash but at the same time this introduces a further regression - up until now the completion worked if a user added some prefix before triggering code completion given that this excluded the missing class from the completion suggestions. I'll see if I manage to somehow bisect the regression

That's weird. I am not sure how such behavior could be explained by the fix in this PR. The fix just filters out alternatives that are referring to missing classes.

@prolativ
Copy link
Contributor

Interestingly, when I tried this now, I could not reproduce the bug that this fix was supposed to introduce anymore, so I think we're good to go

@prolativ prolativ merged commit acbc9aa into scala:main Dec 1, 2022
@prolativ prolativ deleted the fix-16228 branch December 1, 2022 14:30
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.

MissingType crash for code completions
2 participants