diff --git a/clippy_lints/src/atomic_ordering.rs b/clippy_lints/src/atomic_ordering.rs index 2d964ac2b9f6..703d8a6f62bb 100644 --- a/clippy_lints/src/atomic_ordering.rs +++ b/clippy_lints/src/atomic_ordering.rs @@ -8,7 +8,8 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// **What it does:** Checks for usage of invalid atomic - /// ordering in atomic loads/stores and memory fences. + /// ordering in atomic loads/stores/exchanges/updates and + /// memory fences. /// /// **Why is this bad?** Using an invalid atomic ordering /// will cause a panic at run-time. @@ -17,22 +18,35 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust,no_run - /// # use std::sync::atomic::{self, AtomicBool, Ordering}; + /// # use std::sync::atomic::{self, AtomicU8, Ordering}; /// - /// let x = AtomicBool::new(true); + /// let x = AtomicU8::new(0); /// + /// // Bad: `Release` and `AcqRel` cannot be used for `load`. /// let _ = x.load(Ordering::Release); /// let _ = x.load(Ordering::AcqRel); /// - /// x.store(false, Ordering::Acquire); - /// x.store(false, Ordering::AcqRel); + /// // Bad: `Acquire` and `AcqRel` cannot be used for `store`. + /// x.store(1, Ordering::Acquire); + /// x.store(2, Ordering::AcqRel); /// + /// // Bad: `Relaxed` cannot be used as a fence's ordering. /// atomic::fence(Ordering::Relaxed); /// atomic::compiler_fence(Ordering::Relaxed); + /// + /// // Bad: `Release` and `AcqRel` are both always invalid + /// // for the failure ordering (the last arg). + /// let _ = x.compare_exchange(1, 2, Ordering::SeqCst, Ordering::Release); + /// let _ = x.compare_exchange_weak(2, 3, Ordering::AcqRel, Ordering::AcqRel); + /// + /// // Bad: The failure ordering is not allowed to be + /// // stronger than the success order, and `SeqCst` is + /// // stronger than `Relaxed`. + /// let _ = x.fetch_update(Ordering::Relaxed, Ordering::SeqCst, |val| Some(val + val)); /// ``` pub INVALID_ATOMIC_ORDERING, correctness, - "usage of invalid atomic ordering in atomic loads/stores and memory fences" + "usage of invalid atomic ordering in atomic operations and memory fences" } declare_lint_pass!(AtomicOrdering => [INVALID_ATOMIC_ORDERING]); @@ -127,9 +141,89 @@ fn check_memory_fence(cx: &LateContext<'_>, expr: &Expr<'_>) { } } +fn opt_ordering_defid(cx: &LateContext<'_>, ord_arg: &Expr<'_>) -> Option { + if let ExprKind::Path(ref ord_qpath) = ord_arg.kind { + cx.qpath_res(ord_qpath, ord_arg.hir_id).opt_def_id() + } else { + None + } +} + +fn check_atomic_compare_exchange(cx: &LateContext<'_>, expr: &Expr<'_>) { + if_chain! { + if let ExprKind::MethodCall(ref method_path, _, args, _) = &expr.kind; + let method = method_path.ident.name.as_str(); + if type_is_atomic(cx, &args[0]); + if method == "compare_exchange" || method == "compare_exchange_weak" || method == "fetch_update"; + let (success_order_arg, failure_order_arg) = if method == "fetch_update" { + (&args[1], &args[2]) + } else { + (&args[3], &args[4]) + }; + if let Some(fail_ordering_def_id) = opt_ordering_defid(cx, failure_order_arg); + then { + // Helper type holding on to some checking and error reporting data. Has + // - (success ordering name, + // - list of failure orderings forbidden by the success order, + // - suggestion message) + type OrdLintInfo = (&'static str, &'static [&'static str], &'static str); + let relaxed: OrdLintInfo = ("Relaxed", &["SeqCst", "Acquire"], "ordering mode `Relaxed`"); + let acquire: OrdLintInfo = ("Acquire", &["SeqCst"], "ordering modes `Acquire` or `Relaxed`"); + let seq_cst: OrdLintInfo = ("SeqCst", &[], "ordering modes `Acquire`, `SeqCst` or `Relaxed`"); + let release = ("Release", relaxed.1, relaxed.2); + let acqrel = ("AcqRel", acquire.1, acquire.2); + let search = [relaxed, acquire, seq_cst, release, acqrel]; + + let success_lint_info = opt_ordering_defid(cx, success_order_arg) + .and_then(|success_ord_def_id| -> Option { + search + .iter() + .find(|(ordering, ..)| { + match_def_path(cx, success_ord_def_id, + &["core", "sync", "atomic", "Ordering", ordering]) + }) + .copied() + }); + + if match_ordering_def_path(cx, fail_ordering_def_id, &["Release", "AcqRel"]) { + // If we don't know the success order is, use what we'd suggest + // if it were maximally permissive. + let suggested = success_lint_info.unwrap_or(seq_cst).2; + span_lint_and_help( + cx, + INVALID_ATOMIC_ORDERING, + failure_order_arg.span, + &format!( + "{}'s failure ordering may not be `Release` or `AcqRel`", + method, + ), + None, + &format!("consider using {} instead", suggested), + ); + } else if let Some((success_ord_name, bad_ords_given_success, suggested)) = success_lint_info { + if match_ordering_def_path(cx, fail_ordering_def_id, bad_ords_given_success) { + span_lint_and_help( + cx, + INVALID_ATOMIC_ORDERING, + failure_order_arg.span, + &format!( + "{}'s failure ordering may not be stronger than the success ordering of `{}`", + method, + success_ord_name, + ), + None, + &format!("consider using {} instead", suggested), + ); + } + } + } + } +} + impl<'tcx> LateLintPass<'tcx> for AtomicOrdering { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { check_atomic_load_store(cx, expr); check_memory_fence(cx, expr); + check_atomic_compare_exchange(cx, expr); } } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 1468bef02b8a..2718b14e291f 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -930,7 +930,7 @@ pub static ref ALL_LINTS: Vec = vec![ Lint { name: "invalid_atomic_ordering", group: "correctness", - desc: "usage of invalid atomic ordering in atomic loads/stores and memory fences", + desc: "usage of invalid atomic ordering in atomic operations and memory fences", deprecation: None, module: "atomic_ordering", }, diff --git a/tests/ui/atomic_ordering_exchange.rs b/tests/ui/atomic_ordering_exchange.rs new file mode 100644 index 000000000000..1ddc12f9ab21 --- /dev/null +++ b/tests/ui/atomic_ordering_exchange.rs @@ -0,0 +1,45 @@ +#![warn(clippy::invalid_atomic_ordering)] + +use std::sync::atomic::{AtomicUsize, Ordering}; + +fn main() { + // `compare_exchange` (not weak) testing + let x = AtomicUsize::new(0); + + // Allowed ordering combos + let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::Relaxed); + let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::Acquire); + let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::Relaxed); + let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::Relaxed); + let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::Acquire); + let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::Relaxed); + let _ = x.compare_exchange(0, 0, Ordering::SeqCst, Ordering::Relaxed); + let _ = x.compare_exchange(0, 0, Ordering::SeqCst, Ordering::Acquire); + let _ = x.compare_exchange(0, 0, Ordering::SeqCst, Ordering::SeqCst); + + // AcqRel is always forbidden as a failure ordering + let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::AcqRel); + let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::AcqRel); + let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::AcqRel); + let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::AcqRel); + let _ = x.compare_exchange(0, 0, Ordering::SeqCst, Ordering::AcqRel); + + // Release is always forbidden as a failure ordering + let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::Release); + let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::Release); + let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::Release); + let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::Release); + let _ = x.compare_exchange(0, 0, Ordering::SeqCst, Ordering::Release); + + // Release success order forbids failure order of Acquire or SeqCst + let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::Acquire); + let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::SeqCst); + + // Relaxed success order also forbids failure order of Acquire or SeqCst + let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::SeqCst); + let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::Acquire); + + // Acquire/AcqRel forbids failure order of SeqCst + let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::SeqCst); + let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::SeqCst); +} diff --git a/tests/ui/atomic_ordering_exchange.stderr b/tests/ui/atomic_ordering_exchange.stderr new file mode 100644 index 000000000000..4b9bfef79748 --- /dev/null +++ b/tests/ui/atomic_ordering_exchange.stderr @@ -0,0 +1,131 @@ +error: compare_exchange's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:21:57 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::AcqRel); + | ^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::invalid-atomic-ordering` implied by `-D warnings` + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:22:57 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::AcqRel); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: compare_exchange's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:23:57 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::AcqRel); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:24:56 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::AcqRel); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: compare_exchange's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:25:56 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::SeqCst, Ordering::AcqRel); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` instead + +error: compare_exchange's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:28:57 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::Release); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:29:57 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::Release); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: compare_exchange's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:30:57 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::Release); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:31:56 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::Release); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: compare_exchange's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:32:56 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::SeqCst, Ordering::Release); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` instead + +error: compare_exchange's failure ordering may not be stronger than the success ordering of `Release` + --> $DIR/atomic_ordering_exchange.rs:35:57 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::Acquire); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange's failure ordering may not be stronger than the success ordering of `Release` + --> $DIR/atomic_ordering_exchange.rs:36:57 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange's failure ordering may not be stronger than the success ordering of `Relaxed` + --> $DIR/atomic_ordering_exchange.rs:39:57 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange's failure ordering may not be stronger than the success ordering of `Relaxed` + --> $DIR/atomic_ordering_exchange.rs:40:57 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::Acquire); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange's failure ordering may not be stronger than the success ordering of `Acquire` + --> $DIR/atomic_ordering_exchange.rs:43:57 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: compare_exchange's failure ordering may not be stronger than the success ordering of `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:44:56 + | +LL | let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: aborting due to 16 previous errors + diff --git a/tests/ui/atomic_ordering_exchange_weak.rs b/tests/ui/atomic_ordering_exchange_weak.rs new file mode 100644 index 000000000000..590699025072 --- /dev/null +++ b/tests/ui/atomic_ordering_exchange_weak.rs @@ -0,0 +1,47 @@ +#![warn(clippy::invalid_atomic_ordering)] + +use std::sync::atomic::{AtomicPtr, Ordering}; + +fn main() { + let ptr = &mut 5; + let ptr2 = &mut 10; + // `compare_exchange_weak` testing + let x = AtomicPtr::new(ptr); + + // Allowed ordering combos + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::Relaxed); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Acquire, Ordering::Acquire); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Acquire, Ordering::Relaxed); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Release, Ordering::Relaxed); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::AcqRel, Ordering::Acquire); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::AcqRel, Ordering::Relaxed); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::SeqCst, Ordering::Relaxed); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::SeqCst, Ordering::Acquire); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::SeqCst, Ordering::SeqCst); + + // AcqRel is always forbidden as a failure ordering + let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Relaxed, Ordering::AcqRel); + let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Acquire, Ordering::AcqRel); + let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::AcqRel); + let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::AcqRel, Ordering::AcqRel); + let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::SeqCst, Ordering::AcqRel); + + // Release is always forbidden as a failure ordering + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::Release); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Acquire, Ordering::Release); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Release, Ordering::Release); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::AcqRel, Ordering::Release); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::SeqCst, Ordering::Release); + + // Release success order forbids failure order of Acquire or SeqCst + let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::Acquire); + let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::SeqCst); + + // Relaxed success order also forbids failure order of Acquire or SeqCst + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::SeqCst); + let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::Acquire); + + // Acquire/AcqRel forbids failure order of SeqCst + let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Acquire, Ordering::SeqCst); + let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::AcqRel, Ordering::SeqCst); +} diff --git a/tests/ui/atomic_ordering_exchange_weak.stderr b/tests/ui/atomic_ordering_exchange_weak.stderr new file mode 100644 index 000000000000..de7026f3ffaf --- /dev/null +++ b/tests/ui/atomic_ordering_exchange_weak.stderr @@ -0,0 +1,131 @@ +error: compare_exchange_weak's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange_weak.rs:23:67 + | +LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Relaxed, Ordering::AcqRel); + | ^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::invalid-atomic-ordering` implied by `-D warnings` + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange_weak.rs:24:67 + | +LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Acquire, Ordering::AcqRel); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange_weak.rs:25:67 + | +LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::AcqRel); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange_weak.rs:26:66 + | +LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::AcqRel, Ordering::AcqRel); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange_weak.rs:27:66 + | +LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::SeqCst, Ordering::AcqRel); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange_weak.rs:30:67 + | +LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::Release); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange_weak.rs:31:67 + | +LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Acquire, Ordering::Release); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange_weak.rs:32:67 + | +LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Release, Ordering::Release); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange_weak.rs:33:66 + | +LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::AcqRel, Ordering::Release); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange_weak.rs:34:66 + | +LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::SeqCst, Ordering::Release); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be stronger than the success ordering of `Release` + --> $DIR/atomic_ordering_exchange_weak.rs:37:67 + | +LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::Acquire); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be stronger than the success ordering of `Release` + --> $DIR/atomic_ordering_exchange_weak.rs:38:67 + | +LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be stronger than the success ordering of `Relaxed` + --> $DIR/atomic_ordering_exchange_weak.rs:41:67 + | +LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be stronger than the success ordering of `Relaxed` + --> $DIR/atomic_ordering_exchange_weak.rs:42:67 + | +LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::Acquire); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be stronger than the success ordering of `Acquire` + --> $DIR/atomic_ordering_exchange_weak.rs:45:67 + | +LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Acquire, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not be stronger than the success ordering of `AcqRel` + --> $DIR/atomic_ordering_exchange_weak.rs:46:66 + | +LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::AcqRel, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: aborting due to 16 previous errors + diff --git a/tests/ui/atomic_ordering_fetch_update.rs b/tests/ui/atomic_ordering_fetch_update.rs new file mode 100644 index 000000000000..550bdb001e4c --- /dev/null +++ b/tests/ui/atomic_ordering_fetch_update.rs @@ -0,0 +1,45 @@ +#![warn(clippy::invalid_atomic_ordering)] + +use std::sync::atomic::{AtomicIsize, Ordering}; + +fn main() { + // `fetch_update` testing + let x = AtomicIsize::new(0); + + // Allowed ordering combos + let _ = x.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::Acquire, Ordering::Acquire, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::Acquire, Ordering::Relaxed, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::Release, Ordering::Relaxed, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::AcqRel, Ordering::Acquire, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::AcqRel, Ordering::Relaxed, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::SeqCst, Ordering::Relaxed, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::SeqCst, Ordering::Acquire, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::SeqCst, Ordering::SeqCst, |old| Some(old + 1)); + + // AcqRel is always forbidden as a failure ordering + let _ = x.fetch_update(Ordering::Relaxed, Ordering::AcqRel, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::Acquire, Ordering::AcqRel, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::Release, Ordering::AcqRel, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::AcqRel, Ordering::AcqRel, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::SeqCst, Ordering::AcqRel, |old| Some(old + 1)); + + // Release is always forbidden as a failure ordering + let _ = x.fetch_update(Ordering::Relaxed, Ordering::Release, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::Acquire, Ordering::Release, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::Release, Ordering::Release, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::AcqRel, Ordering::Release, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::SeqCst, Ordering::Release, |old| Some(old + 1)); + + // Release success order forbids failure order of Acquire or SeqCst + let _ = x.fetch_update(Ordering::Release, Ordering::Acquire, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::Release, Ordering::SeqCst, |old| Some(old + 1)); + + // Relaxed success order also forbids failure order of Acquire or SeqCst + let _ = x.fetch_update(Ordering::Relaxed, Ordering::SeqCst, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::Relaxed, Ordering::Acquire, |old| Some(old + 1)); + + // Acquire/AcqRel forbids failure order of SeqCst + let _ = x.fetch_update(Ordering::Acquire, Ordering::SeqCst, |old| Some(old + 1)); + let _ = x.fetch_update(Ordering::AcqRel, Ordering::SeqCst, |old| Some(old + 1)); +} diff --git a/tests/ui/atomic_ordering_fetch_update.stderr b/tests/ui/atomic_ordering_fetch_update.stderr new file mode 100644 index 000000000000..694548ece97b --- /dev/null +++ b/tests/ui/atomic_ordering_fetch_update.stderr @@ -0,0 +1,131 @@ +error: fetch_update's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_fetch_update.rs:21:47 + | +LL | let _ = x.fetch_update(Ordering::Relaxed, Ordering::AcqRel, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::invalid-atomic-ordering` implied by `-D warnings` + = help: consider using ordering mode `Relaxed` instead + +error: fetch_update's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_fetch_update.rs:22:47 + | +LL | let _ = x.fetch_update(Ordering::Acquire, Ordering::AcqRel, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: fetch_update's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_fetch_update.rs:23:47 + | +LL | let _ = x.fetch_update(Ordering::Release, Ordering::AcqRel, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: fetch_update's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_fetch_update.rs:24:46 + | +LL | let _ = x.fetch_update(Ordering::AcqRel, Ordering::AcqRel, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: fetch_update's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_fetch_update.rs:25:46 + | +LL | let _ = x.fetch_update(Ordering::SeqCst, Ordering::AcqRel, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` instead + +error: fetch_update's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_fetch_update.rs:28:47 + | +LL | let _ = x.fetch_update(Ordering::Relaxed, Ordering::Release, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: fetch_update's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_fetch_update.rs:29:47 + | +LL | let _ = x.fetch_update(Ordering::Acquire, Ordering::Release, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: fetch_update's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_fetch_update.rs:30:47 + | +LL | let _ = x.fetch_update(Ordering::Release, Ordering::Release, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: fetch_update's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_fetch_update.rs:31:46 + | +LL | let _ = x.fetch_update(Ordering::AcqRel, Ordering::Release, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: fetch_update's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_fetch_update.rs:32:46 + | +LL | let _ = x.fetch_update(Ordering::SeqCst, Ordering::Release, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` instead + +error: fetch_update's failure ordering may not be stronger than the success ordering of `Release` + --> $DIR/atomic_ordering_fetch_update.rs:35:47 + | +LL | let _ = x.fetch_update(Ordering::Release, Ordering::Acquire, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: fetch_update's failure ordering may not be stronger than the success ordering of `Release` + --> $DIR/atomic_ordering_fetch_update.rs:36:47 + | +LL | let _ = x.fetch_update(Ordering::Release, Ordering::SeqCst, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: fetch_update's failure ordering may not be stronger than the success ordering of `Relaxed` + --> $DIR/atomic_ordering_fetch_update.rs:39:47 + | +LL | let _ = x.fetch_update(Ordering::Relaxed, Ordering::SeqCst, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: fetch_update's failure ordering may not be stronger than the success ordering of `Relaxed` + --> $DIR/atomic_ordering_fetch_update.rs:40:47 + | +LL | let _ = x.fetch_update(Ordering::Relaxed, Ordering::Acquire, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: fetch_update's failure ordering may not be stronger than the success ordering of `Acquire` + --> $DIR/atomic_ordering_fetch_update.rs:43:47 + | +LL | let _ = x.fetch_update(Ordering::Acquire, Ordering::SeqCst, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: fetch_update's failure ordering may not be stronger than the success ordering of `AcqRel` + --> $DIR/atomic_ordering_fetch_update.rs:44:46 + | +LL | let _ = x.fetch_update(Ordering::AcqRel, Ordering::SeqCst, |old| Some(old + 1)); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: aborting due to 16 previous errors +