Hacker News Reader

Comment by klysm | Hacker News Reader

A single language level footgun is a pretty amazing feat, at least relative to the current state of the art.

If we're listing language footguns, I would argue that narrowing 'as' is a footgun.

Rust's 'as' is a general cast. It will try to do something reasonable to get from one type to the other, but while a few of the casts are totally unremarkable, some others open up too much opportunity to blow your feet off in my opinion (and I am not alone). Most obviously casting from a larger to a smaller integer is silently truncating. Sometimes, often even, this is what you intended, sometimes it's unimportant, but once in a while it's a trap.

Obviously it's nothing compared to the landmines in some languages, but I think there's room to teach safer options here, warn for and eventually (in a future edition) prohibit the narrowing cast.

Yeah, `as` is pretty much "do what C casts do except without UB" (which is basically "do what Java casts do") but/and people are justifiably uncomfortable with it. It’s very likely to go away (be deprecated) at some point as soon as someone comes up with something explicit-but-ergonomic-enough to supersede it. I’d like to see checked, overflowing, wrapping, and saturating alternatives for narrowing coercions, just like there are for arithmetic operations. Float-to-int coercions have the additional and orthogonal checked/truncating/rounding/floor/ceil axis which complicates things.

You can enable a ton more lints:

    #![warn(clippy::all, clippy::pedantic, clippy::nursery, clippy::cargo)]

This might be too annoying for some people, but you can choose to allow ones that don't matter and keep the rest.

Notably: it will warn for as truncation, integer arithmetic (a+b can panic or overflow; use checked_/wrapping_/saturating_), calling a function in unwrap_or, etc.

Lints: https://rust-lang.github.io/rust-clippy/master/index.html

Agreed. It gets even more crazier with int <-> float "casts" that silently change bits.

another related lifetime footgun involving these RAII guard objects is that the two following lines are not the same:

    let _ = mutex.lock(); // un-used, un-named 'hole' variable
    let _a = mutex.lock(); // un-used, but named 'hole' variable

You might use this when the lock is essentially virtual, guarding something that isn't a data object. The first line will immediately drop the lockguard, while the second will leave it existing until the end of the scope. It's fairly subtle that this is the case.

Where I actually ran into this was with a profiling timer object that was returning suspiciously short timings. But it could just as easily have been a critical section of some sort.

I thought we had a deny by default lint for dropping a Mutex due to a _ binding. If we don't, definitely should.

This https://godbolt.org/z/rx3h1rxW7 does not even warn you that we're not actually locking the mutex.

There's a correct warning if call lock() without using the result, because it has the must-use annotation, but if we throw it into _ the compiler is satisfied even though that's definitely not what a Mutex guard is for.

On the upper righ side, click on tools and select clippy https://play.rust-lang.org/?version=stable&mode=debug&editio... It will produce the following output:

  error: non-binding let on a synchronization lock
   --> src/main.rs:5:5
    |
  5 |     let _ = m.lock();
    |     ^^^^^^^^^^^^^^^^^
    |
    = note: `#[deny(clippy::let_underscore_lock)]` on by default
    = help: consider using an underscore-prefixed named binding or dropping explicitly with `std::mem::drop`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_underscore_lock

Clippy is a third party tool, but highly recommended because of things like this. I would like to see this lint uplifted into rustc.

Firstly, sincerely thanks for telling me how to add Clippy, that would have been a good idea in this context and now I'll remember it's there.

Secondly, yes, I think should be uplifted. I'd even argue for default deny because I can't think of a reason you would want this behaviour and yet not find this more explicit phrasing more maintainable:

  let guard = m.lock();
  Mutex::unlock(guard); // We don't hold the guard, just touch and go

[I'm assuming anybody who actually wants this would prefer Mutex::unlock() over drop() because it says what's going on, and if they care that they can't write this in stable their effort would be appreciated pushing Mutex::unlock stabilisation rather than blocking a default-deny]

tbh I would use (and I think have used) drop() for this just because I know drop exists and what it does but Mutex::unlock is something I'd only ever know about if I went and scanned the whole Mutex page on rustdoc.

I like the idea of these sorts of "drop-with-semantics" functions, but in practice until/unless they add a way to `impl !Drop` (or maybe `#[deprecated]` on a trait impl so you can add a deprecation note on your `impl Drop`) on a type, I'm not sure they're worth it.

you're right, there is a clippy lint for it on a mutex in particular. Tbh I think it might be good to generalize it somehow? Because I think there are other cases it could bite you than just mutexes. But obviously the whole point of doing that in the first place is usually to avoid a must-use warning, so it can't be completely general.

Also, imo, that's probably a clippy lint that should be promoted to a built-in.

Rust has far more than one foot gun, though I do think they’re overall still more minor than many languages I’ve used in the past, which is why I love it so much.

I hope future languages have even fewer. Here’s hoping!

Reply on news.ycombinator.com