Skip to content

Remove implicit tasty.Reflection synthesis #6823

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

Conversation

nicolasstucki
Copy link
Contributor

Replaced by implicit quoted.QuoteContext

@nicolasstucki nicolasstucki force-pushed the remove-implicit-tasty-Reflection-synthesis branch 3 times, most recently from 708f94a to 8743198 Compare July 9, 2019 13:12
@nicolasstucki nicolasstucki marked this pull request as ready for review July 9, 2019 14:19
@nicolasstucki nicolasstucki requested a review from liufengyun July 9, 2019 14:19
Replaced by implicit `quoted.QuoteContext`
@nicolasstucki nicolasstucki force-pushed the remove-implicit-tasty-Reflection-synthesis branch from 8743198 to cc2c34b Compare July 10, 2019 06:16
@nicolasstucki
Copy link
Contributor Author

Rebased

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

Otherwise, LGTM

(formal, span) => implicit ctx =>
if (ctx.inInlineMethod || enclosingInlineds.nonEmpty) ref(defn.TastyReflection_macroContext)
else EmptyTree

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it too early to remove it? Or we may assume that if people are experimenting with new macros, they can tolerate such changes without a transition period?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For most things we already require a QuoteContext with a stable path which this implicit conversion does not provide. Hence using Reflection may easily lead to confusing errors. Having a single way to do things will be clearer. This change also aims to align fully with the documentation and fail where the code is using a pattern that will probably not work at long terms.

@nicolasstucki nicolasstucki merged commit 786dfd9 into scala:master Jul 10, 2019
@nicolasstucki nicolasstucki deleted the remove-implicit-tasty-Reflection-synthesis branch July 10, 2019 08:41
@WojciechMazur WojciechMazur modified the milestone: 3.7.1 May 1, 2025
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