Skip to content
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

Dotty: support distinctions between LTTs of simple subtypes #130

Merged
merged 6 commits into from
Feb 22, 2021

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Jan 31, 2021

e.g. (type T <: String) =:= String should be false (see new test cases)
Seemed the lightest reasonable implementation, but there's a lot of this codebase I don't exactly grok yet.
Doesn't seem to be a ticket that corresponds to this
I also didn't see any similar tests in the commented-out part of the dotty LTT tests, but highly possible I just missed them this doesn't seem to fix any of the commented-out tests so I'm assuming there's either no existing coverage of this, or the coverage that exists uses some much more advanced wizardry

@CLAassistant
Copy link

CLAassistant commented Jan 31, 2021

CLA assistant check
All committers have signed the CLA.

@hughsimpson hughsimpson force-pushed the fix_primitive_subtype_behaviour branch 2 times, most recently from 8e84855 to 48b6533 Compare January 31, 2021 08:30
@hughsimpson
Copy link
Contributor Author

This isn't actually enough to work for my case, on reflection....

Hugh Simpson added 3 commits January 31, 2021 15:13
…ypeBounds thingy in the case of wildcards, where we actually seem to only care about hi after all...
@hughsimpson
Copy link
Contributor Author

This seems to work now. Quite possibly introducing some 🐛, but passes the existing test suite at least

@hughsimpson hughsimpson reopened this Jan 31, 2021
@neko-kai
Copy link
Member

neko-kai commented Feb 2, 2021

@hughsimpson
Thank you for your contributions! 🙏
I want to make sure this change works correctly and is compatible with Scala 2's behavior.

To do that I may suggest to do copy your test to Scala 2's LightTypeTagTest and make sure that output of LightTypeTag#debug is similar in Scala 2 and Scala 3 version of the test.
You may also add tests using assertDebugRepr to fixate that for the future.
Why I'm asking for that is because your change only affects the type boundary stored in LightTypeTagRef, but not the subtyping information stored in basesDb and inheritanceDb. I'm not sure whether the Scala 2 version behaves similarly and whether this would be consistent. If you do find a a disrepancy, you may try to fix it yourself or call for help.

@hughsimpson
Copy link
Contributor Author

Could you explain what the basesDb and inheritanceDb do/represent exactly? TBH this was mostly me hacking around to get something that seemed to work for the precise test cases, but I don't really understand the project terribly well I'm afraid

@neko-kai
Copy link
Member

neko-kai commented Feb 4, 2021

@hughsimpson
basesDb contains inheritance trees of types that are directly mentioned in the type signature that are fully-parameterized and also of their type constructors.
inheritanceDb contains inherinance trees of just the base types / base classes - without type parameters and variance, like Java's type system.

Consider the debug representation of LTT[List[Int]] (on Scala 2) printed using the following code:

      println(LTT[List[Int]].debug())
⚙️ : List[+Int]
⚡️bases: 
 - List[+Int] ->   
   * LinearSeq[+Int]
   * Iterable[+Int]
   * Equals
   * Seq[+Int]
   * Function1[-Int,+Int]
   * DefaultSerializable
   * StrictOptimizedSeqOps[+Int,+λ %1:0 → List[+1:0],+List[+Int]]
   * AbstractSeq[+Int]
   * IterableFactoryDefaults[+Int,+λ %1:0 → List[+1:0]]
   * StrictOptimizedLinearSeqOps[+Int,+λ %1:0 → List[+1:0],+List[+Int]]
   * IterableOnceOps[+Int,+λ %1:0 → List[+1:0],+List[+Int]]
   * StrictOptimizedSeqOps[+Int,+λ %1:0 → List[+1:0],+List[+Int]]
   * PartialFunction[-Int,+Int]
   * LinearSeq[+Int]
   * StrictOptimizedIterableOps[+Int,+λ %1:0 → List[+1:0],+List[+Int]]
   * IterableOnce[+Int]
   * LinearSeqOps[+Int,+λ %1:0 → List[+1:0],+List[+Int]]
   * Serializable
   * AbstractSeq[+Int]
   * Iterable[+Int]
   * LinearSeqOps[+Int,+λ %1:0 → List[+1:0],+List[+Int]]
   * AbstractIterable[+Int]
   * SeqOps[+Int,+λ %1:0 → List[+1:0],+List[+Int]]
   * IterableOps[+Int,+λ %1:0 → List[+1:0],+List[+Int]]
   * Seq[+Int]
   * SeqOps[+Int,+λ %1:0 → List[+1:0],+List[+Int]]
 - λ %0 → List[+0] ->   
   * λ %0 → LinearSeq[+0]
   * λ %0 → IterableOnceOps[+0,+λ %1:0 → List[+1:0],+List[+0]]
   * λ %0 → Iterable[+0]
   * λ %0 → StrictOptimizedIterableOps[+0,+λ %1:0 → List[+1:0],+List[+0]]
   * λ %0 → AbstractSeq[+0]
   * λ %0 → Iterable[+0]
   * λ %0 → StrictOptimizedSeqOps[+0,+λ %1:0 → List[+1:0],+List[+0]]
   * λ %0 → SeqOps[+0,+λ %1:0 → List[+1:0],+List[+0]]
   * λ %0 → LinearSeq[+0]
   * λ %0 → IterableOnce[+0]
   * λ %0 → IterableOps[+0,+λ %1:0 → List[+1:0],+List[+0]]
   * λ %0 → LinearSeqOps[+0,+λ %1:0 → List[+1:0],+List[+0]]
   * λ %0 → PartialFunction[-Int,+0]
   * λ %0 → Function1[-Int,+0]
   * λ %0 → StrictOptimizedSeqOps[+0,+λ %1:0 → List[+1:0],+List[+0]]
   * λ %0 → Seq[+0]
   * λ %0 → AbstractIterable[+0]
   * λ %0 → StrictOptimizedLinearSeqOps[+0,+λ %1:0 → List[+1:0],+List[+0]]
   * λ %0 → IterableFactoryDefaults[+0,+λ %1:0 → List[+1:0]]
   * λ %0 → SeqOps[+0,+λ %1:0 → List[+1:0],+List[+0]]
   * λ %0 → LinearSeqOps[+0,+λ %1:0 → List[+1:0],+List[+0]]
   * λ %0 → AbstractSeq[+0]
   * λ %0 → Seq[+0]
 - Int ->   
   * AnyVal
⚡️inheritance: 
 - List ->   
   * LinearSeq
   * Seq
   * StrictOptimizedLinearSeqOps
   * LinearSeqOps
   * SeqOps
   * AbstractIterable
   * Iterable
   * LinearSeq
   * IterableFactoryDefaults
   * IterableOps
   * PartialFunction
   * SeqOps
   * StrictOptimizedSeqOps
   * Equals
   * DefaultSerializable
   * LinearSeqOps
   * StrictOptimizedSeqOps
   * AbstractSeq
   * Iterable
   * IterableOnceOps
   * IterableOnce
   * StrictOptimizedIterableOps
   * Serializable
   * Seq
   * AbstractSeq
   * Function1
 - Int ->   
   * AnyVal
⚙️ end 

inheritanceDb here contains just the inheritance trees of List & Int, while basesDb is more precise with variance and information that only Int parameterized List is related to AbstractSeq of Int/ SeqOps of Int, List, List/LinearSeq of Int, List[?], List[Int]/etc.

@hughsimpson
Copy link
Contributor Author

Thanks for that. You're absolutely right that there are some issues with this pr as it stands. I'll need a little more time to figure out what the expected distinctions between the debug output should be - a relatively simple fix seems to have solved the prefix issue, but there're quite possibly some deeper issues that I'm gonna have to scope out a bit more. Things are busy at work right now so it will likely be a few days before I'll have time to revisit this. And thanks again for the pointers, really appreciate it - and, of course, really appreciate the library. Moved to it from TypeTags a month or so ago when looking at the 2->3 migration and it has been an absolute win for performance (and jar size!). Will be back here soon when I've figured a bit more out.

… Add some test assertions about the current ref repr, and bases & inheritance maps
@hughsimpson
Copy link
Contributor Author

Pushed up a fix for ref prefix of the case I'm interested in, as well as a few assertions about the inheritance and bases maps. They... behave quite differently to the scala 2 ones. This pr doesn't, as it stands, affect that representation at all...

@neko-kai neko-kai merged commit 53ec02f into zio:develop Feb 22, 2021
@neko-kai
Copy link
Member

@hughsimpson Thanks for fixing this!

@hughsimpson hughsimpson deleted the fix_primitive_subtype_behaviour branch December 17, 2021 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants