Why is rustfmt crashing at all?

#1

I just finished reading about the latest progress on rustfm (Rustfmt-ing Rust ). Having done some work in Go I’m already a fan of automated formatting…so I’m really glad that people put effort into this tool.

The article mentions that rustfmt still suffers from stability problems. Since the program itself is written in Rust, I was wondering what is the cause of these crashes? tried to find uses of unsafe in it’s source-code https://github.com/nrc/rustfmt but there only seems very limited usage of unsafe in the code. Maybe this is a naive question but shouldn’t it be rather easy to prevent crashes? maybe I’m missing something…or maybe the author refers to panics at runtime. Anybody familiar with the problems?

#2

I’m not sure why rustfmt crashes in particular, but to answer this question in a more broader sense.

What Rust will help you address are some kinds of memory problems, and some kinds of concurrency problems. You shouldn’t get a segfault in safe code, but that doesn’t mean you can’t get runtime errors. It also won’t help from memory starvation, or in some sense leaking resources.

Now you might be asking, doesn’t RAII guarantee that all resources are freed? Sure, as long as the object is destroyed. You might still be manipulating the resources in such a way that they never really get freed, because their lifetime is too long (true for all dynamically allocated things). Rust also won’t help you avoid deadlocks, and it doesn’t help prove any kind of correctness of your logic. It also doesn’t help you from problems caused by functions invoked via FFI.

All it does is that it checks some things checkable at compile time. I’m not saying it’s not handy, on quite the contrary, I really like how Rust enforces some things to be done in a proper way (passing things across threads for example).

Last but not least, every program has quite a few invariants, and they can’t all be checked statically (unless you try to prove correctness, in which case you can still crash, because the outside world is evil), so some runtime checks must be implemented using panics.

Sorry for being slightly off topic.

#3

Every rustfmt crash I’ve seen so far was de to an unwrap call without proper error handling (e.g. skipping the code segment). That’s probably just my luck, though :wink:

(If you want to help: Change some unwrap() calls to expect("failure reason")!)

#4

so far that’s also my experience… I checked the source code of rustfmt and there are quite some places where unwrap() is used. So I guess you are right and this causes the program to “crash”.
My initial understanding of the article was that there where actual crashes (like in segfault). But that is probably my background in C++ that is haunting me :wink:

#5

This seems it because you won’t get any backtrace on a segfault:

If Rustfmt crashes during formatting, please get a backtrace by re-running with RUST_BACKTRACE=1.

#6

good point

#7

you are very much on topic…I am really interested in the ways Rust can (and cannot) help with this. So far I only had good experience…so much that it almost seems incredible :sunny:

#8

One small bit of distinction here: While Rust code without unsafe won’t get crashes due to segfaults, it can get crashes due to panics. Panics are better than segfaults, but they can still happen.

Edit: whoops, some people already said this!

#9

yeah…I guess the term “crash” sounds a little different in the ears of rust developers than it does for (mainly) C++ developers

#10

Not necessarily. If you compiled with debugging symbols and don’t have disabled core dumps, a segfault will create a core dump, which you can easily inspect and get a backtrace.

I’m not saying that segfaults are great, but they’re not that hard to diagnose with core dumps. On my system it takes a single command to get to from the crash to the backtrace (coredumpctl gdb foo), which also immediately shows the line at which it crashed (again, given it was compiled with debugging symbols).

#11

For rustfmt, most of the crashes are actually panics due to unwrap(). Another common cause of crashes are integer underflows during computations of width budgets for a line; this problem as been ignored some time, but there is now a standard way to fix it, so it’s getting better quickly.

#12

thanks…good to know! that’s actually the only thing (so far) I tripped over that feels kind of … un-rusty

#13

This is interesting. Long time ago, when .unwrap() was being introduced (unless my memory is failing me), I remember reading a discussion where people said how useful it is, and I remember thinking how bad it is to have an easy syntax to work around error checking (at the time I was using Haskell).

I’m not saying unwrap() is bad, but being so nice to use (compare with things like reinterpret_cast in C++), no wonder things are crashing because it’s used incorrectly.

#14

Haskell has the same thing with fromJust.

unwrap can be useful for initial implementations when you don’t want to think about error handling. The cost is that in the interim, you will pay with runtime crashes and possibly major refactoring when proper error handling is desired.

#15

But at least, compared to catching unused error returns or unhandled exceptions from dozens/hundreds of different functions, it is easy to find unwrap()s that need to be replaced (or just use clippy’s lint for it :smile:).

#16

In order to use fromJust you have to be quite explicit (providing an import), which is what I was trying to argument. Having unwrap() so easily accessible makes it easy to just use it whenever you want, without thinking twice.

From my point of view, unwrap() makes sense only in the case when you absolutely know that there is no way for the value to not be there, but when the type-system can’t know it. That’s why I was saying it should have ugly (visible) syntax that makes you think before using it. Maybe calling it something like unsafe_unwrap(), which I think someone somewhere recommended when the discussion about it was ongoing, would solve the problem of overusing it.

But it’s probably too late for that now, and we have to live with what we have. It might make sense to have a compiler (optional) warning for using unwrap(), with an option to locally disable the warning (in the I-know-what-I’m-doing-here-don’t-bother-me kind of sense).

Maybe I’m being overly critical, but this is probably my least favorite part of all Rust … being so safety-focused, having an easily accessible sort-of-unsafe API that novice users can use without thinking twice will add a lot of unnecessary crashes.

#17

unwrap is not related to unsafety in any way, so it wouldn’t make sense to call it unsafe.

I think it is valuable to be able to write code that explicitly throws away error handling.

It is true that it may be too easy to use. You could in theory propose to deprecate unwrap and replace it with something more obscurely named, but I suspect that will be an uphill battle.

#18

since the possibilities to for such “escape-hatches” are limited (AFAICT only unwrap()) it’s pretty easy to spot and watch out for. Having a linter option in rustc to add warnings would be kind of cool (like rustc -W unwrap) .
In haskell the use of fromJust is possible but discouraged since it is a partial function. (similar to calling head on a possibly empty list)

#19

There’s still the problem of re-architecting your code once you do find the unwrap() that caused trouble. I mean, it’s great that it’s easy to find the place that can cause the error, but that still doesn’t make it easy to handle the error differently.

Consider the case when your default is to use unwrap() and after some testing you try to figure out retrospectively in which cases you actually need to handle the error, and in which cases unwrap() does the trick. You might already have lots of code in place that expects you to go down the happy path, and since Rust doesn’t allow exception handling, you would have to change all of the upstream code to introduce the additional error information, making the intermediary layers aware of possible failures.