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

Disable fatal warnings on 2.12 if cross-building? #378

Closed
armanbilge opened this issue Sep 22, 2022 · 13 comments
Closed

Disable fatal warnings on 2.12 if cross-building? #378

armanbilge opened this issue Sep 22, 2022 · 13 comments

Comments

@armanbilge
Copy link
Member

At least a few Typelevel projects are affected by the changes in scala/scala#9890 (particularly those containing JS facades / Native bindings). Actually that PR improves warnings, but the problem is it is a significant divergence from 2.12. This makes cross-compiling across 2.12/2.13 with fatals enabled on both no longer possible, at least with the default sbt-typelevel-settings.

Some random ideas:

  1. Disable fatals on 2.12 and move on.
  2. Disable just unused warnings on 2.12.
  3. Don't warn for ununused nowarns.
  4. Create a @nowarn212, @nowarn213, and @nowarn3 annotations to help navigate this mess.

/cc @satorg @DavidGregory084 my resident scalac option experts :)

@som-snytt
Copy link

I think -Wconf is a more likely solution.

It would be nice to have a plugin to automate silencing by version, but you're silencing "parameter ... is never used". Or there are "categories" for each flavor. -Wconf:cat="something".

@satorg
Copy link
Contributor

satorg commented Sep 22, 2022

@armanbilge just to clarify: you mean sbt-typelevel 0.5.x line (which is not released yet), but not the 0.4.x one, don't you?
Because in 0.4 all unused warnings are still disabled for 2.12.

@armanbilge
Copy link
Member Author

Oh they are! I completely forgot about that 😂 now I feel silly.

But ok, we still need to think about this for 0.5.x then.

@armanbilge armanbilge added this to the v0.5.0 milestone Sep 22, 2022
@som-snytt
Copy link

all unused warnings are still disabled for 2.12.

I don't use the dis- word anymore, but it would be nice to activate ALL the warnings (insert meme) all the time.

Probably that will be in the form of a sbt-tpolecat that enforces a "profile" per version. An ideal was that -Xlint enforces a baseline.

But maybe it must have the form of -Wconf per version to dial back false positives.

@satorg
Copy link
Contributor

satorg commented Sep 23, 2022

But ok, we still need to think about this for 0.5.x then.

Oh yeah, I do think about it. But the thing is that if we'd like to switch to scalac-option while maintaining the same level of option preciseness, we get to extend the latter with several improvements like allowing a particular option to be adjusted in several ways before being rendered into an option string.

But maybe it must have the form of -Wconf per version to dial back false positives.

Right... There's a great draft proposal in scalac-option to extend it with -Wconf DSL.
Still may require some refinements to be finally incorporated into the lib.

Then, if we manage to add a possibility to tune ScalacOption instance for a particular version range (regardless of whether it is supported or not), then your suggestion will become feasible.

@som-snytt
Copy link

thanks, I'll look up scalac-option as I'm out of the loop. Option precision FTW!

@satorg
Copy link
Contributor

satorg commented Sep 23, 2022

@som-snytt it would be great if you could take a look at typelevel/scalac-options#9
and share your thoughts/suggestions in there. thanks!

@satorg
Copy link
Contributor

satorg commented Sep 27, 2022

@armanbilge is this thread related to your initial concern?

@som-snytt
Copy link

I commented on the linked ticket that it should be trivial to use a less-trivial but trivially efficient syntax.

@DavidGregory084
Copy link
Member

DavidGregory084 commented Sep 27, 2022

Hmm this one is interesting - so just to state the actual problem clearly, the issue is that:

  • The precision of the unused warning was improved in 2.13.9 so that it ignores trivial RHS
  • This is causing @nowarn annotations that were added to silence previous warnings to trigger their own "does not silence any warnings" warning
  • We still get unused warnings for the same code on 2.12 unless we keep the @nowarn annotations, which poses problems for cross-building

I guess the only solution is to add -Wunused:-nowarn or -Wconf:cat=unused-nowarn:silent for 2.13 only and keep the @nowarn annotations? Does that sound right @som-snytt?

@som-snytt
Copy link

The solution on the mouse ticket was to make the RHS less trivial by adding rhs: Unit.

I started looking at why the check for unused @nowarn is difficult or impossible to suppress selectively. If I figure that out, maybe I can get a solution into 2.13.10.

It seems that having unused @nowarn check in -Xlint:unused is the hassle for cross-building. In the absence of selective -Wconf, I agree that -Wunused:-nowarn is simplest way to turn it off.

@armanbilge
Copy link
Member Author

@satorg has implemented Scala-version-specific @nowarn annotations in http4s/http4s#6751.

@armanbilge
Copy link
Member Author

I think scala-compat solves this problem handily. Thank you Sergey!
https://github.com/typelevel/scalac-compat

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

No branches or pull requests

4 participants