Literal/constant-typed local lazy vals need refined values. #6454
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
As discussed, we'll review in Berlin and give ourselves until M5 to refine. |
adriaanm
reviewed
May 18, 2018
| If(initialized, getValue, | ||
| if (isUnit) Block(rhsAtComputer :: Nil, Apply(initialize, Nil)) | ||
| else Apply(initialize, rhsAtComputer :: Nil)))) | ||
| refineLiteral( |
Contributor
There was a problem hiding this comment.
it feels weird the unit case is under the cast. Is there a way to cast a smaller expression?
Compiling the following,
object Test {
type Id[T] = T
def foo(x: Int): Id[x.type] = x
{ lazy val result: 1 = foo(1) }
}
crashes with an unexpected type error during the Fields phase,
error: scala.reflect.internal.Types$TypeError: type mismatch;
found : Int
required: 1
This is because the translation of the local lazy val in Fields maps to
a `LazyInt` which holds an `Int` value. This is too wide to assign to
the `1`-typed val and results in a type error when the tree synthesized
during Fields phase is typechecked.
I've fixed this by inserting a cast to the narrower type, which is
eliminated during erasure. In principle we could switch to a scheme
similar to LazyUnit for literal typed local lazy vals, ie. eliminating
the storage for the value whilst retaining the effects, but this seems
like an unlikely enough scenario to warrant the additional complexity
that this optimization would entail.
This fixes scala/bug#10792.
milessabin
force-pushed
the
topic/si-10792
branch
from
bbca5ab to
1b0106c
Compare
Contributor
Author
|
Narrowed the scope of the refinement, also rebased. |
Contributor
Author
|
@adriaanm this is ready to go now if you're happy. |
adriaanm
approved these changes
Aug 3, 2018
This was referenced Aug 10, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Compiling the following,
crashes with an unexpected type error during the Fields phase,
This is because the translation of the local lazy val in Fields maps to a
LazyIntwhich holds anIntvalue. This is too wide to assign to the1-typed val and results in a type error when the tree synthesizedduring Fields is typechecked.
I've fixed this by inserting a cast to the narrower type, which is eliminated during erasure. In principle we could switch to a scheme similar to
LazyUnitfor literal typed local lazy vals, ie. eliminating the storage for the value whilst retaining the effects, but this seems like an unlikely enough scenario to not warrant the additional complexity that this optimization would entail.This fixes scala/bug#10792.