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

2.12.4 regression (?): Spurious unused warnings #10571

Closed
lloydmeta opened this issue Oct 27, 2017 · 7 comments
Closed

2.12.4 regression (?): Spurious unused warnings #10571

lloydmeta opened this issue Oct 27, 2017 · 7 comments

Comments

@lloydmeta
Copy link

lloydmeta commented Oct 27, 2017

Apologies in advance if this has already been filed.

In 2.12.3, private/local values that are used in macro expansions were considered to be used, but in 2.12.4, they trigger unused value warnings.

This is evident when using something like MacWire to wire up dependencies at compile time.

Repro: https://github.com/lloydmeta/scala-2.12.4-unused-test

Main.scala L6-L12, where wire[T] is used triggers the following compile warning/failures:

[error] ~/scala-2.12.4-unused-test/src/main/scala/com/beachape/Main.scala:6:20: private val config in object Main is never used
[error]   private lazy val config = {
[error]                    ^
[error] ~/scala-2.12.4-unused-test/src/main/scala/com/beachape/Main.scala:7:9: local val host in lazy value config is never used
[error]     val host = "https://my-api.com"
[error]         ^
[error] ~/scala-2.12.4-unused-test/src/main/scala/com/beachape/Main.scala:8:9: local val port in lazy value config is never used
[error]     val port = 9000

Workarounds:

  1. Go back to 2.12.3 in build.sbt
  2. Manually wire up dependencies (e.g. Config(host, port) and new Client(config))
  3. Make all members publicly accessible
@SethTisue
Copy link
Member

SethTisue commented Oct 27, 2017

looks related to #6096. paging @som-snytt?

(Lloyd has clarified that the remaining "2.11"s above should be "2.12"s)

@lloydmeta lloydmeta changed the title 2.11.4 regression (?): Spurious unused warnings 2.12.4 regression (?): Spurious unused warnings Oct 27, 2017
@scala scala deleted a comment from lloydmeta Oct 27, 2017
@som-snytt
Copy link

That is tweaked behavior due to scala/scala#5876

Linting can be done on the unexpanded and/or expanded tree (before/after). It defaults to before (as that represents what was coded directly, as it were).

  "-Xlint",
  "-Ywarn-macros:after",
  "-Ywarn-unused:params,patvars",

@lloydmeta
Copy link
Author

It defaults to before (as that represents what was coded directly, as it were)

Sorry to nitpick, but shouldn't it default to after, given that was the prior behaviour? As it stands, 2.11.4 breaks builds that compiled under 2.11.3.

@SethTisue
Copy link
Member

it's just a warning; we're free to change that stuff around in minor releases, and we routinely do. if you think the new default is poorly chosen, you need a better argument than “it was something else before”

@lloydmeta
Copy link
Author

lloydmeta commented Oct 27, 2017

Nope, was just asking given there wasn't much of an argument for the current behaviour presented. Breaking builds when going up a patch version is generally surprising, but given it's partly caused by turning on fatal warnings, I suppose it's understandable :)

@SethTisue
Copy link
Member

(I guess #10296 was considered self-explanatory, but I haven’t thought about it too deeply)

@som-snytt
Copy link

It's true that someone has to suffer in the name of progress. Compare the linked ticket. The motivation for not linting the expanded tree is that a macro can emit arbitrary cruft, but linting is to help people avoid mistakes. Some kinds of magic aren't lintable at all, for example generating different code depending on whether an implicit value is in scope (like language features). It's been suggested to let a macro tell the compiler, "I used this symbol."

An improvement might be to look only for usages in the expanded tree, ignoring newly introduced cruft. That would be a different mode from "it passes lint before and after."

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

No branches or pull requests

3 participants