Skip to content

Improve presentation compiler with Shapeless macros #5929

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

Merged
merged 1 commit into from
Jun 26, 2017

Conversation

retronym
Copy link
Member

@retronym retronym commented Jun 7, 2017

-Ymacro-expand discard was introduced to leave prefixes and
arguments of implicit applications in the typechecked tree in the
presentation compiler to facilite completion, type-at-cursor, etc
within those trees.

It seems that this mode interferes with implicit searches that involve
Shapeless's mkLazy macro.

This commit simply turns off the macro expansion discarding if
the current macro application is part of an application of implicit
arguments. There is no trees corresponding to source code in that
position, so we the concerns about IDE functionality are moot.

I couldn't disentangle the bug report from circe and shapeless,
so I've manually tested that the compiler from this commit
makes the following test project compile.

https://github.com/retronym/t9716

Fixes scala/bug#9716

@scala-jenkins scala-jenkins added this to the 2.12.3 milestone Jun 7, 2017
`-Ymacro-expand` discard was introduced to leave prefixes and
arguments of implicit applications in the typechecked tree in the
presentation compiler to facilite completion, type-at-cursor, etc
within those trees.

It seems that this mode interferes with implicit searches that involve
Shapeless's `mkLazy` macro.

This commit simply turns off the macro expansion discarding if
the currnt macro application is part of an application of implicit
arguments. There is no trees corresponding to source code in that
position, so we the concerns about IDE functionality are moot.

I couldn't disentangle the bug report from circe and shapeless,
so I've manually tested that the compiler from this commit
makes the following test project compile.

https://github.com/retronym/t9716
@retronym retronym changed the title Don't discard macro expansions in implicit param position Improve presentation compiler with Shapeless macros Jun 7, 2017
@retronym
Copy link
Member Author

retronym commented Jun 7, 2017

@mpollmeier @milessabin Perhaps you could give the combination of this and #5927 a spin in Ensime (instructions) to see if it makes life better? Be sure to disable the any manual setting of -Ymacro-expand:normal in the presentation compiler options that might have been added as a workaround for this bug.

/cc @fommil

@fommil
Copy link
Contributor

fommil commented Jun 7, 2017

we use discard https://github.com/ensime/ensime-sbt/blob/2.0/src/main/scala/EnsimePlugin.scala#L219 but only because somebody told me to do it. What should we be using?

BTW, somewhat related me and @dragos started writing https://github.com/ensime/ensime-plugin-implicits but ran out of time. This will have huge benefits for presentation compiler users in implicit-heavy codebases.

@retronym
Copy link
Member Author

retronym commented Jun 7, 2017 via email

@mpollmeier
Copy link
Contributor

@retronym I tested with ensime and now the autocompletion works fine in both cases :)
I didn't have to change anything other than the scala jars in .ensime. As @fommil said, ensime uses -Ymacro-expand:discard by default.

@fommil
Copy link
Contributor

fommil commented Jun 7, 2017

awesome, looking forward to this! @mpollmeier if you'd like to help out with presentation compiler testing / performance there are a few things I have in mind, none of them particularly onerous 😄

@adriaanm adriaanm merged commit 80bfdee into scala:2.12.x Jun 26, 2017
@retronym retronym mentioned this pull request Jul 7, 2017
37 tasks
@retronym retronym added the release-notes worth highlighting in next release notes label Jul 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants