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.