-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Intrinsify scala.compiletime.testing.typeChecks #7129
Intrinsify scala.compiletime.testing.typeChecks #7129
Conversation
bd2f838
to
f4ba907
Compare
@liufengyun could you help me to update the scalatest macros to use this new version? Have a look at how I handled it in tests/run-macros/reflect-typeChecks/assert_1.scala |
Is this going to break shapeless as well? I'm happy to update as soon as there's a nightly build published with the change. |
Shapeless should not be affected. CI tests passed for it. |
I will update minitest. With this change we will not need the macro anymore as it fixed #7040 and we can use that as implementation. |
@nicolasstucki I'll have a look at ScalaTest. |
@nicolasstucki This change makes it awkward to implement macros like On reflection, it seems BTW, what makes it not fit in reflection API?
|
As know from the start, this funtionallity should not have been added to the reflection API. Nevertheless it is still possible to use its result in a macro.
048accd
to
48d88bc
Compare
It does not fit the TASTy reflection API because it has nothing to do with typed trees. We just added it there because it was a simple but dirty way to support it and unblock the scalatest/minitest macros. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR decides that typeChecks
should only be used in inline methods instead of macros.
Approve based on the following reasons:
- the impact is limited AFAWK (some ScalaTest DSL has to be redesigned),
- it's always possible to restore the API when there is strong need.
As know from the start, this funtionallity should not have been added to the reflection API.
Nevertheless it is still possible to use its result in a macro.