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

Make shapeless.the respect @implicitNotFound (Fix #671) #672

Closed
wants to merge 3 commits into from

Conversation

Atry
Copy link
Contributor

@Atry Atry commented Jan 3, 2017

No description provided.

@codecov-io
Copy link

codecov-io commented Jan 3, 2017

Current coverage is 81.56% (diff: 100%)

Merging #672 into master will not change coverage

@@             master       #672   diff @@
==========================================
  Files            69         69          
  Lines          2641       2641          
  Methods        2375       2375          
  Messages          0          0          
  Branches         92         92          
==========================================
  Hits           2154       2154          
  Misses          487        487          
  Partials          0          0          

Powered by Codecov. Last update fcc15a6...6768f0f

@milessabin
Copy link
Owner

Great stuff! I think that slient = false means that the following isEmpty condition will always be false ... could you check whether or not that's the case and if it is remove the conditional.

@Atry
Copy link
Contributor Author

Atry commented Jan 11, 2017

I removed the conditional and left an assert call instead

@milessabin
Copy link
Owner

Thanks! I think the assert should go though ... it's not validating any logic of ours, so doesn't really belong here.

@milessabin
Copy link
Owner

Sorry to keep chiselling away on this ... would you mind adding a test which verifies that the yields values with refined type members as described in the scaladoc?

@milessabin
Copy link
Owner

Bump ... I'll merge as soon as the additional test has been added.

@Atry
Copy link
Contributor Author

Atry commented Feb 13, 2017

Thank you.

@Atry
Copy link
Contributor Author

Atry commented Mar 14, 2017

Don't know how to test the error message

@milessabin
Copy link
Owner

Add a test of the form,

val foo = ...
foo: <type including refinement>

This will verify that the inferred type of foo is the expected type, including the expected refinement, without the refinement influencing type inference. Alternatively used assertTypedEquals.

@Atry
Copy link
Contributor Author

Atry commented Mar 14, 2017

This PR only changes the error message. How to test if the error message is expected?

@milessabin
Copy link
Owner

Whoops! Sorry, I'm confusing PRs :-)

You should be able to use illTyped with an expected error message here.

Please also squash the resulting commits.

@AndreasKostler
Copy link
Contributor

AndreasKostler commented Jul 26, 2017

I am not sure this does what it's expected to do (according to #671):

type Foo = the.`Ordering[Set[Int]]`.Ops
error: exception during macro expansion:
scala.reflect.macros.TypecheckException: implicit search has failed. to find out the reason, turn on -Xlog-implicits

expected

error:  No implicit Ordering defined for Set[Int]

@AndreasKostler
Copy link
Contributor

AndreasKostler commented Jul 26, 2017

I don't want to hijack this PR but I do have a working one if interested. See #742

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants