Stream: t-compiler/rust-analyzer

Topic: PSA: always assert never


matklad (Jan 14 2021 at 15:28, on Zulip):

Hey, I suggest changing the strategy of asserts in rust-analyzer: https://github.com/rust-analyzer/rust-analyzer/pull/7270

matklad (Jan 14 2021 at 15:29, on Zulip):

TL;DR: instead of assert!(cond), we now write

matklad (Jan 14 2021 at 15:29, on Zulip):

(deleted)

matklad (Jan 14 2021 at 15:29, on Zulip):
if stdx::assert_never!(!cond) {
    handle_impossible()
}
Florian Diebold (Jan 14 2021 at 15:31, on Zulip):

I like it, it splits the difference between "this should never happen and I want to check that" and "but it'd actually be pretty easy to do something sensible in this situation", which is a dilemma that I have all the time...

matklad (Jan 14 2021 at 15:32, on Zulip):

yay fearless assertivness!

Lukas Wirth (Jan 14 2021 at 19:20, on Zulip):

I think you might've forgotten to push the assert_always macro with it, the assert_never macro references it in the first transcriber but it doesn't exist :sweat_smile: https://github.com/rust-analyzer/rust-analyzer/commit/8dc68ecdfcc764c7c0dcf5fcedcb51b092d99620#diff-4ab2b49eae91be2ce0307d9a427fb0cacf505aec4881b742d9bee842d580e81bR69

matklad (Jan 14 2021 at 19:46, on Zulip):

argh

matklad (Jan 14 2021 at 19:47, on Zulip):

it's rather that I was lazy to push implement both macros, and botched a rename from always to never

matklad (Jan 14 2021 at 19:48, on Zulip):

noting defeats the advantages of a strong static type system quite as effectively as a powerful macro machinery

matklad (Jan 21 2021 at 08:53, on Zulip):

cc @Jack Huey, I think long-term it might make sense to use something like this for chalk? We hit assertion errors fairly often, and, long term, they shouldn't crash the whole request.

OTOH, right now crashing rust-analyzer might be nice, because it forces users to come up with sweat minimal examples: https://github.com/rust-analyzer/rust-analyzer/issues/6956#issuecomment-764459620

Florian Diebold (Jan 21 2021 at 10:20, on Zulip):

I don't know, e.g. the "wrong number of type parameters substituted into a type" indicates a rather severe bug somewhere, and at the point of that assertion it's a pretty deep assumption that's very hard to continue without

matklad (Jan 21 2021 at 10:34, on Zulip):

That's also true. Hm, don't we wrap chalk into catch_unwind already? I guess then it doesn't really matter

Florian Diebold (Jan 21 2021 at 10:40, on Zulip):

I don't think we do anymore

Florian Diebold (Jan 21 2021 at 10:41, on Zulip):

I guess we could handle it similar to an assert_never around the whole solver call

Jack Huey (Jan 21 2021 at 14:28, on Zulip):

I'm with @Florian Diebold here: it would be really hard to recover from most of the errors we encounter. It might be worth introducing some kind of FatalError state that can be returned instead of a solution (or lack of). Then try to bubble that up. But that would mean refactoring basically the entire codebase. At that point, it's probably easier to just panic/catch_unwind

Last update: Jul 29 2021 at 08:00UTC