Consider removing `mutex_atomic`

[carllerche]

There are many reasons to use mutexes over atomics. Clippy is unable to correctly determine if an atomic is sufficient to replace the mutex use. Attempting to lint over could lead to pushing users to introduce bugs in their code.

I recommend removing this lint.

[Manishearth]

I feel like this lint is correct most of the time; and clippy lints are supposed to be used with a liberal sprinkling of allow(). There needs to be stronger, more detailed justification for removing a lint.

[carllerche]

My main point is the potential harm of taking action based on a false positive is high compared to other provided lints.

[carllerche]

(I can't comment on the rate of false positive besides saying I have only experienced a 100% false positive rate 😄 ).

[Manishearth]

I disagree, lints aren't as strong signals as compiler warnings, and Clippy's warnings are meant to be taken with a grain of salt. Here the signal is "there might be an issue", if you determine it isn't an issue, allow() it.

If you really think there's a problem here a stronger case needs to be made, with lots of examples.

[Manishearth]

I have only experienced a 100% false positive rate

Like I said, clippy is supposed to be used with a liberal sprinkling of allow(). Some lints just don't make sense for some codebases. It's not possible for our lints to be totally globally applicable. It's up to the codebase to make this judgement.

[carllerche]

Ok, feel free to close if you disagree.

[Manishearth]

(leaving open since other maintainers, and other people, may have different opinions)

[flip1995]

The lint mentions in its Known problems section that

This lint cannot detect if the mutex is actually used for waiting before a critical section.

I'm with @Manishearth here. I think in the general case this lint is correct and Atomics are sufficient. If you really need the mutex you should just allow the lint.

Removing this lint would require to show (with examples), that you need a Mutex in the general case.

[thombles]

I want to add a +1 since I ran into this reviewing a PR the other day where the submitter defaulted to using Relaxed. Replacing Mutex<bool> with AtomicBool like it shows in the example is easy, but now you need to reason correctly about Ordering. Speaking only for myself, I've read about three articles about memory ordering and I still don't know which combination of Acquire and Release to use in a given situation. I happen to know that using SeqCst everywhere is safe, maybe because I read that on stack overflow once. That's a whole lot of homework that goes unmentioned and if you don't follow through on it you might introduce heisenbugs.

I'm quite happy to use the allow feature as noted above but I feel like the lint is a footgun to begin with. Those who know how to use atomics correctly will notice this potential performance issue the moment they see it.

[taiki-e]

Filed #8260 to downgrade this lint to nursery group that allowed by default.

Seems that Carl originally proposed to remove this lint completely, but the maintainers rejected it, so I believe downgrading is a reasonable compromise for now.