-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Generate mirrors for named tuples #22469
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
Conversation
1251ae6 to
9077d4f
Compare
| // avoid type aliases for tuples | ||
| Right(MirrorSource.GenericTuple(types)) | ||
| case _ => reduce(tp.underlying) | ||
| case tp @ AppliedType(tref: TypeRef, List(_, typeTuple)) if tref == defn.NamedTupleTypeRef => |
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.
| case tp @ AppliedType(tref: TypeRef, List(_, typeTuple)) if tref == defn.NamedTupleTypeRef => | |
| case tp @ AppliedType(tref: TypeRef, List(_, _)) if tref == defn.NamedTupleTypeRef => |
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 ended up changing it into defn.NamedTupleDirect(_, _) (same AppliedType internally), for readability (I don't think that unapply was there when I started making this PR)
| Right(MirrorSource.GenericTuple(types)) | ||
| case _ => reduce(tp.underlying) | ||
| case tp @ AppliedType(tref: TypeRef, List(_, typeTuple)) if tref == defn.NamedTupleTypeRef => | ||
| Right(MirrorSource.NamedTuple(tp.namedTupleElementTypes(false))) |
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.
what changes with derived=true instead of false?
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.
derived=true allows to convert union and intersection types of NamedTuples into NamedTuple AppliedType types. We already know it's a NamedTupleDirect from the unapply above, so we don't need to do that (and setting true would not change anything here).
| val (labels, typeElems) = nameTypePairs.unzip | ||
| val elemLabels = labels.map(label => ConstantType(Constant(label.toString))) | ||
| val mirrorRef: Type => Tree = _ => newTupleMirror(typeElems.size) | ||
| makeProductMirror(typeElems, elemLabels, "NamedTuple".toTypeName, mirrorRef) |
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.
| makeProductMirror(typeElems, elemLabels, "NamedTuple".toTypeName, mirrorRef) | |
| makeProductMirror(typeElems, elemLabels, tpnme.NamedTuple, mirrorRef) |
if this doesnt exist it should
| ("", NoType, cls) | ||
| case Right(MirrorSource.NamedTuple(nameTypePairs)) => | ||
| val arity = nameTypePairs.size | ||
| val cls = if arity <= Definitions.MaxTupleArity then defn.TupleType(arity).nn.classSymbol else defn.PairClass |
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.
| val cls = if arity <= Definitions.MaxTupleArity then defn.TupleType(arity).nn.classSymbol else defn.PairClass |
unused? (is there a side-effect?)
| val cls = if arity <= Definitions.MaxTupleArity then defn.TupleType(arity).nn.classSymbol else defn.PairClass | ||
| ("", NoType, cls) | ||
| case Right(MirrorSource.NamedTuple(nameTypePairs)) => | ||
| val arity = nameTypePairs.size |
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.
(also unused)
9077d4f to
0a6e367
Compare
Fixes #22382
For
summon[Mirror.Of[(foo: Int, bla: String)]]we generate:We reuse scala.runtime.TupleMirror, because it pretty much does everything we want it to, and fromProduct (with supplied Product types) call on that mirror still works there.
Since NamedTuple is not technically a
Producttype, I imagine users might be a little confused why they can't put a named tuple into afromProductargument, but this is easily worked around with.toTuple