Skip to content

Put implicit support for evidence from predef types#3508

Merged
johnynek merged 3 commits intomasterfrom
oscar/evidence_improvement
Jul 5, 2020
Merged

Put implicit support for evidence from predef types#3508
johnynek merged 3 commits intomasterfrom
oscar/evidence_improvement

Conversation

@johnynek
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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