Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix failing bounds check on default getter #18419

Merged
merged 1 commit into from
Aug 21, 2023

Conversation

dwijnand
Copy link
Member

@dwijnand dwijnand commented Aug 17, 2023

The type arguments on the default argument had the wrong span, which
means that they were getting bounds checked. This is in contrast to the
other type arguments (the ones on check itself), which aren't
bounds checked - by not passing the isZeroExtent guard in
checkInferredWellFormed.

The type arguments on the default argument had the wrong span, which
means that they were getting bounds checked.  This is in contrast the
other other type arguments (the ones on check itself), which aren't
bounds checked  - by not passing the isZeroExtent guard in
checkInferredWellFormed.
@dwijnand dwijnand linked an issue Aug 17, 2023 that may be closed by this pull request
@dwijnand dwijnand requested a review from odersky August 18, 2023 07:39
@dwijnand dwijnand marked this pull request as ready for review August 18, 2023 07:39
@@ -333,7 +333,7 @@ object Applications {
// it's crucial that the type tree is not copied directly as argument to
// `cpy$default$1`. If it was, the variable `X'` would already be interpolated
// when typing the default argument, which is too early.
spliceMeth(meth, fn).appliedToTypes(targs.tpes)
spliceMeth(meth, fn).appliedToTypeTrees(targs.map(targ => TypeTree(targ.tpe).withSpan(targ.span)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this type of confusion occur somewhere else in code, but we just didn't notice it yet?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't understand the question. By code, do you mean compiler code, or do you mean something about user's code?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I wasn't clear. I meant in compiler code where we have something like .appliedToTypes(targs.tpes) and actually needs to be constructed with span.

Copy link
Member Author

@dwijnand dwijnand Aug 18, 2023

Choose a reason for hiding this comment

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

Not that I'm aware of. Looking now, perhaps the .appliedToTypes(patternTypes) in quotes pattern should reuse the original pattern tree spans, but I'm not certain. I was thinking Martin would be able to tell if this is a reoccurring problem and how to best deal with it.

@Kordyjan

This comment was marked as outdated.

Copy link
Contributor

@odersky odersky left a 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 a corner case (default getters have a hang of producing those). Cannot see at the moment other situations that would require a generalization.

@odersky odersky merged commit 3212890 into scala:main Aug 21, 2023
17 checks passed
@soronpo
Copy link
Contributor

soronpo commented Aug 21, 2023

While this fixes the minimized example, it does not affect the DFiant library, which still fails with the same Recursion limit exceeded error on collectParts.

The recursion limit bug is a different issue: #18263

@dwijnand dwijnand deleted the inferred-targ-def-arg branch August 22, 2023 20:29
@Kordyjan Kordyjan added this to the 3.4.0 milestone Dec 20, 2023
WojciechMazur added a commit that referenced this pull request Jun 20, 2024
Backports #18419 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
WojciechMazur added a commit that referenced this pull request Jun 20, 2024
Backports #18419 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upper bound lost from Int to Any on Scala 3.1.2 and later
4 participants