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

Put implicit support for evidence from predef types #3508

Merged
merged 3 commits into from
Jul 5, 2020

Conversation

johnynek
Copy link
Contributor

@johnynek johnynek commented Jul 5, 2020

close #2569

There is no reason not to create Is and As instances from the predef ones. The changes in 2.13 make this perfectly safe, but nothing in 2.12 (or before) made it unsafe, the types just lacked a substitute method.

This PR improves matters a bit.

I did notice that Is is not sealed, but As is. In general these two types have a pretty different API considering how similar they are conceptually.

I only did a bit, I added toPredef to both of them to convert back to predef types.

travisbrown
travisbrown previously approved these changes Jul 5, 2020
Copy link
Contributor

@travisbrown travisbrown left a comment

Choose a reason for hiding this comment

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

👍 when green.

* value.
*/
@inline final def toPredef: A <:< B =
substitute[<:<[*, B]](implicitly[B <:< B])
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs the same treatment as above? Something like:

type F[-Z] = Z <:< B
substitute[F](implicitly[B <:< B])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, you are right! I got impatient because on my slow machine the tests took too long. I've fixed the tests on 2.12 and 2.13 so I think it will be green now.

Thank you for the review and thanks for considering it for 2.2.0.

@travisbrown travisbrown added this to the 2.2.0-RC1 milestone Jul 5, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #3508 into master will decrease coverage by 0.01%.
The diff coverage is 75.00%.

@@            Coverage Diff             @@
##           master    #3508      +/-   ##
==========================================
- Coverage   91.75%   91.74%   -0.02%     
==========================================
  Files         383      383              
  Lines        8406     8418      +12     
  Branches      232      220      -12     
==========================================
+ Hits         7713     7723      +10     
- Misses        693      695       +2     

Copy link
Member

@LukaJCB LukaJCB left a comment

Choose a reason for hiding this comment

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

LGTM thanks Oscar!

@johnynek johnynek merged commit 040e008 into master Jul 5, 2020
@johnynek johnynek deleted the oscar/evidence_improvement branch July 5, 2020 23:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cats.evidence.Is and Scala 2.13
5 participants