Adopt the configurable warning mechanism from Scala 2.13.2

[smarter]

See scala/scala#8373

[iamrecursion]

Is the way it's designed for scalac in that PR a good fit for the dotty codebase? I'm wondering to what extent implementing this for dotty will be able to crib code and design, beyond replicating the functionality itself.

[smarter]

Our reporting mechanisms are fairly similar, so I would say yes, but I haven't looked at it in details.

[smarter]

Dotty has one extra interesting thing: messages which are written as case classes (#1589) get a unique ID (https://github.com/lampepfl/dotty/blob/master/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala), this could be used to filter messages precisely.

[iamrecursion]

That sounds like a really nice augmentation of what the warnings suppression system can already do.

From using it in 2.13.2, one of the biggest awkwardnesses is that warnings are printed after errors for some types of compilation error. I'd imagine dotty doesn't need to maintain CLI output parity, and hence can potentially do better in this regard?

[smarter]

Normally, the order in which errors/warnings are reported is the order in which the compiler encounter them in both scalac and dotty. Does the configurable warning suppression mechanism affect that?

[iamrecursion]

Due to the design of the functionality, the suppression mechanism doesn't always work and you get unexpected output:

From scala/scala#8373

If we stop before typer (e.g., because there are errors in the parser), all warnings are shown, even if they are enclosed in @nowarn

[smarter]

Hmm, but even when there are parser errors, both scalac and dotty try to recover from them (e.g., by inserting "null") and run the typer anyway, not sure in what situation that wouldn't work. But it's true that if you need to wait until typer to know if a warning should be displayed, then all parser warnings need to be buffered so I see why they could appear after parser errors. The alternative would be to handle @nowarn directly in the parser but that might be tricky to do (and incorrect in general, since the user is free to write import unrelatedthing.nowarn, and then @nowarn would have a different meaning, but we don't know that until we actually resolve it in the typer). So I don't know if there's really a better alternative to what scalac is doing here.

[smarter]

@lrytz For scala/scala#8373, did you consider treating @nowarn specially in the parser, to avoid having to buffer warning messages? It would mean that a different annotation importer under the same name would behave incorrectly, but that doesn't seem like a huge price to pay.

[iamrecursion]

I think it was code involving macros where I saw it, but I can't remember exactly.

[lrytz]

I did consider it, but decided for the symbolic approach for correctness (eg import rename). But I agree it's a trade off, and the other approach is also vaible. You'd have to know the position range of the subtree covered by a @nowarn before actually parsing the subtree

@nowarn def foo: Unit = 1
def bar: Unit = 1: @nowarn

Are there scanner warnings?

[smarter]

Are there scanner warnings?

Yeah, but not the kind that makes sense to disable as far as I can see: https://github.com/lampepfl/dotty/blob/9a47eca862ea28e93467dbe7f20c34f02a552f5e/compiler/src/dotty/tools/dotc/parsing/Scanners.scala#L252, https://github.com/lampepfl/dotty/blob/9a47eca862ea28e93467dbe7f20c34f02a552f5e/compiler/src/dotty/tools/dotc/parsing/Scanners.scala#L389, https://github.com/lampepfl/dotty/blob/9a47eca862ea28e93467dbe7f20c34f02a552f5e/compiler/src/dotty/tools/dotc/parsing/Scanners.scala#L513

[iamrecursion]

It does sound like keeping the existing approach taken in Scala 2 is the best one for correctness, then.

[som-snytt]

It's almost the weekend and school is out for the summer (or forever), so I'll take a look.

I see there is already interest in keeping awesome messages as simple as possible, and I'm also interested in minimal ceremony around kinds/categories.

I would also be interested in backporting to Scala 2. Next weekend. Sample previous comment on rendering, custom linting, tool advice such as Run again with -explain.

Probably I've jinxed myself and it will be another two years before I remember to do this.

[OlivierBlanvillain]

I see there is already interest in keeping awesome messages as simple as possible, and I'm also interested in minimal ceremony around kinds/categories.

There is interest in getting rid of them. These messages are a pain to maintain and pretty low quality at the moment. IMO "-explain" is not a nice mechanism to get help on an error message. That's better solved by a Google/StackOverflow query or even an IDE tooltip...

[som-snytt]

For me, the awesome part is the awesome sauce, namely, the formatting.

I didn't know there were strong opinions over here about the messages. Or wait, I did read "heavy and impractical" at the end of the ticket.

I tried -explain once this evening and created a ticket because I was confused. I wanted to help with the messages effort a couple of years ago but only got as far as reading the instruction manual.

[OlivierBlanvillain]

I didn't know there were strong opinions over here about the messages.

Yes, we discussed it several times during in-person meeting. The conclusion is that editing your sbt build to get extra info about an error message is a terrible workflow, and that the dotty team does not have the bandwidth to create and maintain these extra explanations. If somebody makes a PR that gets rid of all these extra error messages and the infrastructure that comes with them it would get merged.

[som-snytt]

I'm taking advantage of the pandemic heat wave and long weekend to look at this. Summer isn't over until it falls below 40C.

[prolativ]

@som-snytt are you currently working on this issue? If not then I might take it over

[som-snytt]

I gave it more thought -- it looks like I have a branch. Oh, I see the requisite "multivalued setting" thing was merged, so I could take this up again. The branch says I introduced the Wconf stuff. I could push something if there is enthusiasm.

There are two takes: compiler should not be in the business of emitting warnings; but if third-party linters are popular, the standard reporter could filter their noise in a standard way.

I have not yet thought about the explain bit, namely, direct the user to a web page which explains stuff. Something like this except it would be, Why I am so 🤦 .

My other question (in this area) was whether dotty wants to adopt scala 2.13 convention of -V -W -X -Y. That got some love from Seth, but I never heard the vox populi. I found it made scala 2 options easier, but maybe scala 2 has too many options.

commit 5dfd95e4afcd6b3255afa41f7339c7bc7e266a46 (HEAD -> issue/8908-wconf)
Author: Som Snytt <[email protected]>
Date:   Mon Sep 7 23:30:04 2020 -0700

    Handle some wconf

commit 9e34eb47f76d9f5909c6f8f4c98e0216c5cd2504
Author: Som Snytt <[email protected]>
Date:   Mon Sep 7 10:04:46 2020 -0700

    tweak

commit 5fff4f5ec00449e68ee89f8a9a752e23fc9ad67b
Author: Som Snytt <[email protected]>
Date:   Sun Sep 6 21:19:02 2020 -0700

    Wconf option with help text

commit ae2c4abd2b0baf9bc45de2d19cff9f0ea216d25c
Author: Som Snytt <[email protected]>
Date:   Fri Sep 4 17:10:20 2020 -0700

    Multivalued setting can be set multiply

[smarter]

My other question (in this area) was whether dotty wants to adopt scala 2.13 convention of -V -W -X -Y.

When in doubt, aligning ourselves with Scala 2 is a good idea I think.

[SethTisue]

I think adoption of -V and -W out in the wild has been somewhat slow, but I think it's natural there would be inertia there, for multiple reasons: people aren't motivated to change what ain't broken; OSS maintainers typically cross-build and are motivated to stick with flags that are accepted by the oldest Scala version they cross-build for; and I think that a lot of people simply aren't aware of the new forms, largely (IMO) because we haven't yet started warning on the obsolete forms.

So yes, we still consider the new forms the gold standard for Scala 2, and we hope that Scala 3 will follow suit.

I think it would be super if Scala 3 started actively suggesting the new forms, and personally I'd support a PR that made Scala 2 start to do it, too, though I don't know what the rest of the Lightbend team thinks about that.

[som-snytt]

Nothing takes your mind off the US electoral process like a dotty issue, so I'll try to PR something that someone can adopt if desired.

I noted (#7575 (comment)) that the unfortunate position on -language:_ could be re-jiggered so that -language just aliases -Wconf. That is, some language constructs may be OK, or warnings, or errors. Picking -language just selects a set of such configurations. But -Wconf is a post-processing step, so it may not work to manage scanner/lexer differences.

[smarter]

-language does more than turning off/on warnings in dotty, it can also affect semantics like with -language:strictEquality, #9884 would also introduce -language:unsafeNulls

[prolativ]

@som-snytt what's the status of this? Is choosing the proper compiler setting name the only blocker?
If think it might be quite important to reproduce scala2's functionality before 3.0.0-RC1. Otherwise users who are used to treating warnings as errors might be reluctant to switch to scala3. @smarter what do you think?

[smarter]

The sooner the better of course, but I don't think it's critical for RC1.

[som-snytt]

@prolativ I did a little bit on -Wconf and some more on "align compiler options with scala 2". I didn't do work on reporters yet, so you can pick it up if you like. I know Martin doesn't favor warnings, so I think the important use case is migration warnings. I have some day job obligations. I didn't look further at -language as merely warning suppression.

[prolativ]

@som-snytt have you pushed the changes to some branch? Then maybe I could have a look

[som-snytt]

@prolativ sorry for the delay responding, I had a bout of illness. Also sorry, now I see you asked me a month ago. I will PR #10236 with the wconf from september.

Edit: I rebased those couple of commits, in case it's useful. Feel free to use it without attribution. I'm having trouble finding time to get back to it; I might during the holidays. https://github.com/som-snytt/dotty/tree/issue/10236

[smarter]

Edit: I rebased those couple of commits, in case it's useful. Feel free to use it without attribution. I'm having trouble finding time to get back to it; I might during the holidays. https://github.com/som-snytt/dotty/tree/issue/10236

Thanks @som-snytt, this looks really useful!

[lrytz]

https://github.com/som-snytt/dotty/tree/issue/10236

Rebased: https://github.com/lampepfl/dotty/compare/master...lrytz:wconf?expand=1

I'm planning to work on wconf in the next days.

[som-snytt]

@lrytz thanks. The three plates (in the air) were compiler options and addressing "heavy weight" of error messages and wconf categories. I don't know if there was consensus on option style; I noticed sadly that the migration tool just normalizes your build if you were using the -VW style. I did not give much design thought to making the wconf lighter weight in terms of introducing categories; is there consensus that it's great when you know the wconf string to use, less great if you don't, or you're introducing a category when adding an error.