Skip to content

Dotty pickles type trees of | and & to applied type tree of fictional symbols #7688

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

Closed
bishabosha opened this issue Dec 5, 2019 · 9 comments
Labels
area:pickling area:tasty-format issues relating to TASTy as a portable standard itype:bug

Comments

@bishabosha
Copy link
Member

minimized code

Because intersection types and union types are pickled in TASTy to applied type trees of imaginary types scala.& and scala.|, all implementations must intercept these names, when they could be reified as the intrinsic types that they are.

trait A
trait B

type AwithB = A with B // pickles to scala.&[A,B]
type AandB  = A & B    // pickles to scala.&[A,B]

type AorB = A | B      // pickles to scala.|[A,B]

expectation

maybe new tags ANDtpt and ORtpt

@smarter
Copy link
Member

smarter commented Dec 5, 2019

These tags were actually removed in #5935 but it might make sense to bring them back, the main benefit of #5935 is that it simplifies dotty internals, but that doesn't mean we can't represent & and | explicitly in Tasty itself. Also I think that this PR removed all the logic in tasty-reflect for constructing & and | types which is something people actually want to do in macros (cf. https://contributors.scala-lang.org/t/union-types-and-generalisations/3820/6?u=smarter) /cc @nicolasstucki

@bishabosha
Copy link
Member Author

I took a stab at this, but preserving the positions for the idempotency test seems hard

@smarter
Copy link
Member

smarter commented Dec 11, 2019

do you mean that the positions are lost somewhere ?

@bishabosha
Copy link
Member Author

bishabosha commented Dec 11, 2019

So I pickled just the type arguments of the AppliedTypeTree, with ANDtpt length tag, then when I reconstruct an AppliedTypeTree from the args and a ref(defn.andType), the range was off by one

@bishabosha
Copy link
Member Author

so I need to preserve the position of the tycon that is thrown away

@smarter
Copy link
Member

smarter commented Dec 11, 2019

Right, the typetree itself should have a position, but I don't think that's any different from other type trees.

@smarter
Copy link
Member

smarter commented Dec 11, 2019

Or do you mean the position of & itself in A & B ? I don't think that one matters for anything (you can't rename & to something else) so preserving it is not important.

bishabosha added a commit to dotty-staging/dotty that referenced this issue Dec 11, 2019
@bishabosha
Copy link
Member Author

$ diff before-pickling.txt after-pickling.txt
53c53
<                 ):scala.quoted.Expr[$t0 & T2@tests/pos/i7264c.scala<113..116>]@tests/pos/i7264c.scala<113..116>@
---
>                 ):scala.quoted.Expr[$t0 & T2@tests/pos/i7264c.scala<114..116>]@tests/pos/i7264c.scala<113..116>@

bishabosha added a commit to dotty-staging/dotty that referenced this issue Dec 13, 2019
@bishabosha bishabosha added the area:tasty-format issues relating to TASTy as a portable standard label Jan 17, 2020
@bishabosha
Copy link
Member Author

closing because we discussed in a meeting about tasty format that scala.| and scala.& are legitimate members of the scala package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:pickling area:tasty-format issues relating to TASTy as a portable standard itype:bug
Projects
None yet
Development

No branches or pull requests

2 participants