compile stdlib & compiler with fatal-warnings / deprecations

[adriaanm]

this will likely require some error filtering, but it would be good to clean house a little in the warning department

[som-snytt]

Maybe Snow White could get all the little forest animals to help.

[SethTisue]

configurable warnings has just landed, but we won't be able to start using it ourselves until we re-STARR on 2.13.2

[soronpo]

I might get in on this action. How is it preferable to submit PRs for this? One big one with separate commit for every silencing? Separate PRs for batches of silencing that differ in type of warning?

[SethTisue]

also see what others have to say, but I was vaguely imagining going module by module, with the goal of being able to enable -Werror module by module as well. (probably with an easy switch to flip to interactively disable it during development)

[dwijnand]

👨‍🎨 let's only enable -Werror in CI, where it matters, as it's (IMHO) a needless inhibition during dev to not run tests because of an unused import somewhere. Whether that's if CI then add or if localDev then remove I don't mind.

1 PR with a commit per silencing type, IMO. E.g. commit-1: silence this warning for reason X - here's the warning output before, commit-2: silence this warning for reason Y - here's the warning output before, etc.

[SethTisue]

we have now re-STARRed on 2.13.2, so we're free to progress on this now

My own first PR in this area has now been merged: scala/scala#8964 . Further PRs like this from contributors are strongly encouraged!

let's only enable -Werror in CI, where it matters

This remains an open question. I agree that it at minimum, it ought to be verrrrry easy to be more lenient when doing local development. I'm not sure what I think about what the default ought to be. If during local development all it took to switch to lenient mode were a single command (yolo), that might be okay?

To have to remember to switch would admittedly be annoying, but the alternative has its own annoying-ness as well: you submit a PR that was fine locally, and then all of a sudden CI is like, ohhhhh no you don't.

[NthPortal]

To have to remember to switch would admittedly be annoying, but the alternative has its own annoying-ness as well: you submit a PR that was fine locally, and then all of a sudden CI is like, ohhhhh no you don't.

Having experienced this at work, I can say that your PR suddenly failing when it built locally is annoying; at the same time, some build tools (e.g. VSC with Metals) become difficult or frustrating to use when you can't even temporarily have errors. For example, if you lint the various "unused"s, you'll get unused import errors constantly during development.

[som-snytt]

Having a PR fail because of -Werror is almost as annoying as failing because of bootstrapping.

Similar problem still to solve is transient warnings in REPL. Maybe it will be to show * marker on result line to mean warnings, or warning on third attempt.

[soronpo]

Having experienced this at work, I can say that your PR suddenly failing when it built locally is annoying; at the same time, some build tools (e.g. VSC with Metals) become difficult or frustrating to use when you can't even temporarily have errors. For example, if you lint the various "unused"s, you'll get unused import errors constantly during development.

Then perhaps it's possible to make it so that only the official CI will reject a build with warnings.

[NthPortal]

Then perhaps it's possible to make it so that only the official CI will reject a build with warnings.

It is, and that's what we do at work (by default; you can enable fatal warnings locally if you want). It still ends up being annoying either way though.

[dwijnand]

• Clean up warnings in library module scala#9235,
• Clean up warnings and deprecate inner class shadowing in reflect module scala#9237, and
• Clean up warnings in compiler module scala#9242

Thank you, @NthPortal!