-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix #21669: Check parents non-empty before calling reduceLeft #21673
Conversation
Hi @odersky @EugeneFlesselle , I saw you worked on #20084. Do you think an empty
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other thing we could do is harden import sugestions. We catch NonFatal when computing a suggestion. We could also do the same when comparing suggestions. That's where the previous error happens.
@@ -1962,7 +1962,9 @@ trait Applications extends Compatibility { | |||
|
|||
def widenPrefix(alt: TermRef): Type = alt.prefix.widen match | |||
case pre: (TypeRef | ThisType) if pre.typeSymbol.is(Module) => | |||
pre.parents.reduceLeft(TypeComparer.andType(_, _)) | |||
val ps = pre.parents | |||
if ps.isEmpty then pre |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it's an invariant that parents of modules are non-empty. except if the module is a package. We should not ignore that invariant here. So before returning pre, let's add the assertion
assert(pre.typeSymbol.is(Package), pre)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, that's an invariant of if the module has been entered. But it could be wrong before. Anyway, it's an important invariant so I think we should not ignore it here, and catch any errors in importSuggestions instead.
Hi @WojciechMazur , since we don't have a regression test currently, can you help testing this PR on CB to double check that it doesn't break anything? Thanks! |
We haven't found any new regressions in the OpenCB when compared with 3.6.0-RC1-bin-20241002-c332766-NIGHTLY |
Does the PR fix the original problem reported in #21669? |
Yes, I have tested it locally following the step I described. |
Fix #21669
This part of code is from #20084
We should check
parents
is non-empty before callingreduceLeft
.I haven't been able to create a standalone test. To test locally:
i21669.scala