Stream: t-compiler

Topic: What's the best way to report fatal errors?


matklad (Aug 14 2019 at 18:57, on Zulip):

At the moment, lexer panics on some errors.

I think panicking is bad for compiler (it certainly is bad for IDEs), and I'd like to fix it. What's the correct way of doing it?

I would think that I should still report a diagnostic with Fatal severity, but just avoid actually raising FatalError. However, #[must_use] on FatalError seems to hint that, if you produced a Fatal diagnostics, you must panic.

Should I just produce an Error? Should I use PhaseFatal?

If FatalError always panics and we want to move away from panicing, should we add a comment saying "don't add new usages of FatalError and try to remove existing ones"?

simulacrum (Aug 14 2019 at 18:57, on Zulip):

IIRC, there's a way to call something like exit_if_errors?

simulacrum (Aug 14 2019 at 18:58, on Zulip):

That's probably what you want after emitting just a regular error

matklad (Aug 14 2019 at 18:58, on Zulip):

abort_if_errors, yeah

simulacrum (Aug 14 2019 at 18:59, on Zulip):

Yeah, I think that should work, right? Or is there something I'm missing? Presumably FatalError.raise() could just call abort_if_errors (modulo global session and such, but we can thread it through I think)

matklad (Aug 14 2019 at 18:59, on Zulip):

That doesn't discriminate between recoverable errors and "oh noes, don't even try to run further phases of the compiler to avoid the flood of cascading errors"

matklad (Aug 14 2019 at 19:00, on Zulip):

FatalError.raise() could just call abort_if_errors (modulo global session and such, but we can thread it through I think)

It's the other way arround: abort_if_errors calls FatalError.raise() :D

centril (Aug 14 2019 at 19:01, on Zulip):

I assume in this case there's no choice but to fatally abort?

simulacrum (Aug 14 2019 at 19:01, on Zulip):

hm, but you implied FatalError.raise() panics?

simulacrum (Aug 14 2019 at 19:01, on Zulip):

which I didn't think was the case

matklad (Aug 14 2019 at 19:02, on Zulip):

Sorry for the sloppy wording: FatalError.raise unwinds (via resume unwind, not via panic), and I assume that we don't actually want that behavior for "something's wrong with user's code"

matklad (Aug 14 2019 at 19:04, on Zulip):

@centril on the contrary, in the lexer's case, we can just print an error, produce a fake token and chug along. However that might trigger cascading errors.

centril (Aug 14 2019 at 19:04, on Zulip):

do we need an ::Err token to avoid cascading errors?

centril (Aug 14 2019 at 19:04, on Zulip):

cc @Vadim Petrochenkov @Esteban Küber

Esteban Küber (Aug 14 2019 at 19:15, on Zulip):

It wouldnt help in the lexer

Esteban Küber (Aug 14 2019 at 19:16, on Zulip):

Or rather, you need the lexer to return an Err for cascading errors not to happen

matklad (Aug 14 2019 at 19:17, on Zulip):

What about Level::PhaseFatal? Semantically, it is exactly what I need here, but, although this variant exists, it isn't actually used anywhere =/ There's an irony in that Level::PhaseFatal is the only documented error variant

matklad (Aug 14 2019 at 19:18, on Zulip):

@Esteban Küber returning Result is no-go for IDE use-case. IDEs should basically never stop processing. Cascading errors is not a problem if you emit squiggly underlines, and it's important to make at least some sense of the code after the error.

matklad (Aug 14 2019 at 19:18, on Zulip):

For example, here's how Kotlin deals with unclosed strings:

pasted image

matklad (Aug 14 2019 at 19:19, on Zulip):

there's a bunch of errors (8), but they are not that harmful, b/c they are not printed to the terminal, and IDE does useful stuff for the following declaration

nikomatsakis (Aug 14 2019 at 19:31, on Zulip):

I've found returning Results to be super annoying in practice

nikomatsakis (Aug 14 2019 at 19:31, on Zulip):

it seems like sentinel is better overall

centril (Aug 14 2019 at 19:33, on Zulip):

@nikomatsakis the approaches seem isomorphic tho? -- at least with Result you get monad powers

oli (Aug 14 2019 at 19:34, on Zulip):

In a compiler I wrote over the last weeks I experimented without sentinels and marked the spans as "already errored" and silenced further warnings on spans with such markings

simulacrum (Aug 14 2019 at 19:42, on Zulip):

Result doesn't work well for "log and continue" I think, since you end up writing equivalent of unwrap_or(sentinel) anyway essentially

nikomatsakis (Aug 14 2019 at 19:44, on Zulip):

The approaches may seem isomorphic but, in general, I found it much smoother to have (e.g.) an Err variant inside of Ty than to make Ty always be Result<Ty, Error> (and so forth for other types). Maybe, if you were thorough, they would both work out roughly the same.

centril (Aug 14 2019 at 19:45, on Zulip):

Maybe Result is the wrong monad; http://hackage.haskell.org/package/monad-validate-1.2.0.0/docs/Control-Monad-Validate.html seems interesting; Something more Writer-monad like might be better

centril (Aug 14 2019 at 19:48, on Zulip):

@nikomatsakis Yeah I suppose it's a question of biasing towards handling or not handling the error case. Having ::Err inside might also be better for introducing (possibly named) typed holes and whatnot later

Last update: Nov 16 2019 at 01:05UTC