From 6211599ccafee5b2429bfb1bbeb48ead32a48484 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 9 Sep 2020 14:00:31 -0700 Subject: [PATCH 1/7] Extend invalid_atomic_ordering to detect misuse of compare_exchange{,_weak} --- clippy_lints/src/atomic_ordering.rs | 82 ++++++- tests/ui/atomic_ordering_exchange.rs | 84 ++++++++ tests/ui/atomic_ordering_exchange.stderr | 259 +++++++++++++++++++++++ 3 files changed, 423 insertions(+), 2 deletions(-) create mode 100644 tests/ui/atomic_ordering_exchange.rs create mode 100644 tests/ui/atomic_ordering_exchange.stderr diff --git a/clippy_lints/src/atomic_ordering.rs b/clippy_lints/src/atomic_ordering.rs index 2d964ac2b9f..748f45f47c2 100644 --- a/clippy_lints/src/atomic_ordering.rs +++ b/clippy_lints/src/atomic_ordering.rs @@ -8,7 +8,7 @@ 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 and memory fences /// /// **Why is this bad?** Using an invalid atomic ordering /// will cause a panic at run-time. @@ -29,10 +29,13 @@ declare_clippy_lint! { /// /// atomic::fence(Ordering::Relaxed); /// atomic::compiler_fence(Ordering::Relaxed); + /// + /// let _ = x.compare_exchange(false, false, Ordering::Relaxed, Ordering::SeqCst); + /// let _ = x.compare_exchange_weak(false, true, Ordering::SeqCst, Ordering::Release); /// ``` pub INVALID_ATOMIC_ORDERING, correctness, - "usage of invalid atomic ordering in atomic loads/stores and memory fences" + "usage of invalid atomic ordering in atomic loads/stores/exchanges ane memory fences" } declare_lint_pass!(AtomicOrdering => [INVALID_ATOMIC_ORDERING]); @@ -127,9 +130,84 @@ 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"; + let failure_order_arg = &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, &args[3]) + .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 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/tests/ui/atomic_ordering_exchange.rs b/tests/ui/atomic_ordering_exchange.rs new file mode 100644 index 00000000000..2f60a98f037 --- /dev/null +++ b/tests/ui/atomic_ordering_exchange.rs @@ -0,0 +1,84 @@ +#![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); + + // compare_exchange_weak tests + + // Allowed ordering combos + let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::Relaxed); + let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::Acquire); + let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::Relaxed); + let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::Relaxed); + let _ = x.compare_exchange_weak(0, 0, Ordering::AcqRel, Ordering::Acquire); + let _ = x.compare_exchange_weak(0, 0, Ordering::AcqRel, Ordering::Relaxed); + let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::Relaxed); + let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::Acquire); + let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::SeqCst); + + // AcqRel is always forbidden as a failure ordering + let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::AcqRel); + let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::AcqRel); + let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::AcqRel); + let _ = x.compare_exchange_weak(0, 0, Ordering::AcqRel, Ordering::AcqRel); + let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::AcqRel); + + // Release is always forbidden as a failure ordering + let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::Release); + let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::Release); + let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::Release); + let _ = x.compare_exchange_weak(0, 0, Ordering::AcqRel, Ordering::Release); + let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::Release); + + // Release success order forbids failure order of Acquire or SeqCst + let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::Acquire); + let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::SeqCst); + + // Relaxed success order also forbids failure order of Acquire or SeqCst + let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::SeqCst); + let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::Acquire); + + // Acquire/AcqRel forbids failure order of SeqCst + let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::SeqCst); + let _ = x.compare_exchange_weak(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 00000000000..26ec15be518 --- /dev/null +++ b/tests/ui/atomic_ordering_exchange.stderr @@ -0,0 +1,259 @@ +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 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 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 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 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 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 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: compare_exchange_weak's failure ordering may not be `Release` or `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:60:62 + | +LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, 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.rs:61:62 + | +LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:62:62 + | +LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:63:61 + | +LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:64:61 + | +LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:67:62 + | +LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:68:62 + | +LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:69:62 + | +LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:70:61 + | +LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:71:61 + | +LL | let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::Release); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Release` + --> $DIR/atomic_ordering_exchange.rs:74:62 + | +LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::Acquire); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Release` + --> $DIR/atomic_ordering_exchange.rs:75:62 + | +LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Relaxed` + --> $DIR/atomic_ordering_exchange.rs:78:62 + | +LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Relaxed` + --> $DIR/atomic_ordering_exchange.rs:79:62 + | +LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::Acquire); + | ^^^^^^^^^^^^^^^^^ + | + = help: consider using ordering mode `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Acquire` + --> $DIR/atomic_ordering_exchange.rs:82:62 + | +LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `AcqRel` + --> $DIR/atomic_ordering_exchange.rs:83:61 + | +LL | let _ = x.compare_exchange_weak(0, 0, Ordering::AcqRel, Ordering::SeqCst); + | ^^^^^^^^^^^^^^^^ + | + = help: consider using ordering modes `Acquire` or `Relaxed` instead + +error: aborting due to 32 previous errors + From 61671a2268903e1b5c28fcb3c713c27e84ea3e9b Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 9 Sep 2020 14:12:14 -0700 Subject: [PATCH 2/7] Detect fetch_update misuse in invalid_atomic_ordering too --- clippy_lints/src/atomic_ordering.rs | 16 ++- src/lintlist/mod.rs | 2 +- tests/ui/atomic_ordering_fetch_update.rs | 45 +++++++ tests/ui/atomic_ordering_fetch_update.stderr | 131 +++++++++++++++++++ 4 files changed, 188 insertions(+), 6 deletions(-) create mode 100644 tests/ui/atomic_ordering_fetch_update.rs create mode 100644 tests/ui/atomic_ordering_fetch_update.stderr diff --git a/clippy_lints/src/atomic_ordering.rs b/clippy_lints/src/atomic_ordering.rs index 748f45f47c2..ff2c281ec9d 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/exchanges 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. @@ -32,10 +33,11 @@ declare_clippy_lint! { /// /// let _ = x.compare_exchange(false, false, Ordering::Relaxed, Ordering::SeqCst); /// let _ = x.compare_exchange_weak(false, true, Ordering::SeqCst, Ordering::Release); + /// let _ = x.fetch_update(Ordering::AcqRel, Ordering::AcqRel, |val| Some(val ^ val)); /// ``` pub INVALID_ATOMIC_ORDERING, correctness, - "usage of invalid atomic ordering in atomic loads/stores/exchanges ane memory fences" + "usage of invalid atomic ordering in atomic operations and memory fences" } declare_lint_pass!(AtomicOrdering => [INVALID_ATOMIC_ORDERING]); @@ -142,8 +144,12 @@ fn check_atomic_compare_exchange(cx: &LateContext<'_>, expr: &Expr<'_>) { 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"; - let failure_order_arg = &args[4]; + 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 @@ -158,7 +164,7 @@ fn check_atomic_compare_exchange(cx: &LateContext<'_>, expr: &Expr<'_>) { let acqrel = ("AcqRel", acquire.1, acquire.2); let search = [relaxed, acquire, seq_cst, release, acqrel]; - let success_lint_info = opt_ordering_defid(cx, &args[3]) + let success_lint_info = opt_ordering_defid(cx, success_order_arg) .and_then(|success_ord_def_id| -> Option { search .iter() diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index dff19ef440f..5dead1b9b69 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -923,7 +923,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_fetch_update.rs b/tests/ui/atomic_ordering_fetch_update.rs new file mode 100644 index 00000000000..550bdb001e4 --- /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 00000000000..362e104a244 --- /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 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 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 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 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 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 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 + From 159178e5d4a023f3945b504b49d3742b40453fee Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 9 Sep 2020 14:32:38 -0700 Subject: [PATCH 3/7] Separate compare_exchange and compare_exchange_weak uitests --- tests/ui/atomic_ordering_exchange.rs | 39 ------ tests/ui/atomic_ordering_exchange.stderr | 130 +---------------- tests/ui/atomic_ordering_exchange_weak.rs | 47 +++++++ tests/ui/atomic_ordering_exchange_weak.stderr | 131 ++++++++++++++++++ 4 files changed, 179 insertions(+), 168 deletions(-) create mode 100644 tests/ui/atomic_ordering_exchange_weak.rs create mode 100644 tests/ui/atomic_ordering_exchange_weak.stderr diff --git a/tests/ui/atomic_ordering_exchange.rs b/tests/ui/atomic_ordering_exchange.rs index 2f60a98f037..1ddc12f9ab2 100644 --- a/tests/ui/atomic_ordering_exchange.rs +++ b/tests/ui/atomic_ordering_exchange.rs @@ -42,43 +42,4 @@ fn main() { // 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); - - // compare_exchange_weak tests - - // Allowed ordering combos - let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::Relaxed); - let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::Acquire); - let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::Relaxed); - let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::Relaxed); - let _ = x.compare_exchange_weak(0, 0, Ordering::AcqRel, Ordering::Acquire); - let _ = x.compare_exchange_weak(0, 0, Ordering::AcqRel, Ordering::Relaxed); - let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::Relaxed); - let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::Acquire); - let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::SeqCst); - - // AcqRel is always forbidden as a failure ordering - let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::AcqRel); - let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::AcqRel); - let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::AcqRel); - let _ = x.compare_exchange_weak(0, 0, Ordering::AcqRel, Ordering::AcqRel); - let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::AcqRel); - - // Release is always forbidden as a failure ordering - let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::Release); - let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::Release); - let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::Release); - let _ = x.compare_exchange_weak(0, 0, Ordering::AcqRel, Ordering::Release); - let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::Release); - - // Release success order forbids failure order of Acquire or SeqCst - let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::Acquire); - let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::SeqCst); - - // Relaxed success order also forbids failure order of Acquire or SeqCst - let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::SeqCst); - let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::Acquire); - - // Acquire/AcqRel forbids failure order of SeqCst - let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::SeqCst); - let _ = x.compare_exchange_weak(0, 0, Ordering::AcqRel, Ordering::SeqCst); } diff --git a/tests/ui/atomic_ordering_exchange.stderr b/tests/ui/atomic_ordering_exchange.stderr index 26ec15be518..c09d2d6ab21 100644 --- a/tests/ui/atomic_ordering_exchange.stderr +++ b/tests/ui/atomic_ordering_exchange.stderr @@ -127,133 +127,5 @@ LL | let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::SeqCst); | = 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.rs:60:62 - | -LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, 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.rs:61:62 - | -LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:62:62 - | -LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:63:61 - | -LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:64:61 - | -LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:67:62 - | -LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:68:62 - | -LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:69:62 - | -LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:70:61 - | -LL | let _ = x.compare_exchange_weak(0, 0, 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.rs:71:61 - | -LL | let _ = x.compare_exchange_weak(0, 0, Ordering::SeqCst, Ordering::Release); - | ^^^^^^^^^^^^^^^^^ - | - = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` instead - -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Release` - --> $DIR/atomic_ordering_exchange.rs:74:62 - | -LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::Acquire); - | ^^^^^^^^^^^^^^^^^ - | - = help: consider using ordering mode `Relaxed` instead - -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Release` - --> $DIR/atomic_ordering_exchange.rs:75:62 - | -LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Release, Ordering::SeqCst); - | ^^^^^^^^^^^^^^^^ - | - = help: consider using ordering mode `Relaxed` instead - -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Relaxed` - --> $DIR/atomic_ordering_exchange.rs:78:62 - | -LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::SeqCst); - | ^^^^^^^^^^^^^^^^ - | - = help: consider using ordering mode `Relaxed` instead - -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Relaxed` - --> $DIR/atomic_ordering_exchange.rs:79:62 - | -LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Relaxed, Ordering::Acquire); - | ^^^^^^^^^^^^^^^^^ - | - = help: consider using ordering mode `Relaxed` instead - -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Acquire` - --> $DIR/atomic_ordering_exchange.rs:82:62 - | -LL | let _ = x.compare_exchange_weak(0, 0, Ordering::Acquire, Ordering::SeqCst); - | ^^^^^^^^^^^^^^^^ - | - = help: consider using ordering modes `Acquire` or `Relaxed` instead - -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `AcqRel` - --> $DIR/atomic_ordering_exchange.rs:83:61 - | -LL | let _ = x.compare_exchange_weak(0, 0, Ordering::AcqRel, Ordering::SeqCst); - | ^^^^^^^^^^^^^^^^ - | - = help: consider using ordering modes `Acquire` or `Relaxed` instead - -error: aborting due to 32 previous errors +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 00000000000..59069902507 --- /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 00000000000..7210431bd1b --- /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 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 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 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 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 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 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 + From b65745545f410b31c5ecdd33a8299a65da578af2 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 9 Sep 2020 14:40:43 -0700 Subject: [PATCH 4/7] Use AtomicU8 in ordering example so all operations can be demonstrated --- clippy_lints/src/atomic_ordering.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/atomic_ordering.rs b/clippy_lints/src/atomic_ordering.rs index ff2c281ec9d..d4e070b0c24 100644 --- a/clippy_lints/src/atomic_ordering.rs +++ b/clippy_lints/src/atomic_ordering.rs @@ -18,22 +18,22 @@ 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); /// /// let _ = x.load(Ordering::Release); /// let _ = x.load(Ordering::AcqRel); /// - /// x.store(false, Ordering::Acquire); - /// x.store(false, Ordering::AcqRel); + /// x.store(1, Ordering::Acquire); + /// x.store(2, Ordering::AcqRel); /// /// atomic::fence(Ordering::Relaxed); /// atomic::compiler_fence(Ordering::Relaxed); /// - /// let _ = x.compare_exchange(false, false, Ordering::Relaxed, Ordering::SeqCst); - /// let _ = x.compare_exchange_weak(false, true, Ordering::SeqCst, Ordering::Release); - /// let _ = x.fetch_update(Ordering::AcqRel, Ordering::AcqRel, |val| Some(val ^ val)); + /// let _ = x.compare_exchange(1, 2, Ordering::Relaxed, Ordering::SeqCst); + /// let _ = x.compare_exchange_weak(2, 3, Ordering::SeqCst, Ordering::Release); + /// let _ = x.fetch_update(Ordering::AcqRel, Ordering::AcqRel, |val| Some(val + val)); /// ``` pub INVALID_ATOMIC_ORDERING, correctness, From 4b5326b0d62104801f0b33c4d8f3749d93eebc02 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 9 Sep 2020 23:12:57 -0700 Subject: [PATCH 5/7] Address small review comments --- clippy_lints/src/atomic_ordering.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/atomic_ordering.rs b/clippy_lints/src/atomic_ordering.rs index d4e070b0c24..614e33a06da 100644 --- a/clippy_lints/src/atomic_ordering.rs +++ b/clippy_lints/src/atomic_ordering.rs @@ -139,6 +139,7 @@ fn opt_ordering_defid(cx: &LateContext<'_>, ord_arg: &Expr<'_>) -> Option None } } + fn check_atomic_compare_exchange(cx: &LateContext<'_>, expr: &Expr<'_>) { if_chain! { if let ExprKind::MethodCall(ref method_path, _, args, _) = &expr.kind; @@ -197,7 +198,7 @@ fn check_atomic_compare_exchange(cx: &LateContext<'_>, expr: &Expr<'_>) { INVALID_ATOMIC_ORDERING, failure_order_arg.span, &format!( - "{}'s failure ordering may not stronger than the success ordering of `{}`", + "{}'s failure ordering may not be stronger than the success ordering of `{}`", method, success_ord_name, ), From 3a072131da2e574e914073af6d72360ead735781 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Wed, 9 Sep 2020 23:22:01 -0700 Subject: [PATCH 6/7] Ah, right, rerun the scripts --- tests/ui/atomic_ordering_exchange.stderr | 12 ++++++------ tests/ui/atomic_ordering_exchange_weak.stderr | 12 ++++++------ tests/ui/atomic_ordering_fetch_update.stderr | 12 ++++++------ 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/ui/atomic_ordering_exchange.stderr b/tests/ui/atomic_ordering_exchange.stderr index c09d2d6ab21..4b9bfef7974 100644 --- a/tests/ui/atomic_ordering_exchange.stderr +++ b/tests/ui/atomic_ordering_exchange.stderr @@ -79,7 +79,7 @@ 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 stronger than the success ordering of `Release` +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); @@ -87,7 +87,7 @@ 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 stronger than the success ordering of `Release` +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); @@ -95,7 +95,7 @@ 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 stronger than the success ordering of `Relaxed` +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); @@ -103,7 +103,7 @@ 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 stronger than the success ordering of `Relaxed` +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); @@ -111,7 +111,7 @@ 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 stronger than the success ordering of `Acquire` +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); @@ -119,7 +119,7 @@ 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 stronger than the success ordering of `AcqRel` +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); diff --git a/tests/ui/atomic_ordering_exchange_weak.stderr b/tests/ui/atomic_ordering_exchange_weak.stderr index 7210431bd1b..de7026f3ffa 100644 --- a/tests/ui/atomic_ordering_exchange_weak.stderr +++ b/tests/ui/atomic_ordering_exchange_weak.stderr @@ -79,7 +79,7 @@ LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::SeqCst, Ordering:: | = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` instead -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Release` +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); @@ -87,7 +87,7 @@ LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering: | = help: consider using ordering mode `Relaxed` instead -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Release` +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); @@ -95,7 +95,7 @@ LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering: | = help: consider using ordering mode `Relaxed` instead -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Relaxed` +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); @@ -103,7 +103,7 @@ LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering: | = help: consider using ordering mode `Relaxed` instead -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Relaxed` +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); @@ -111,7 +111,7 @@ LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering: | = help: consider using ordering mode `Relaxed` instead -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `Acquire` +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); @@ -119,7 +119,7 @@ LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Acquire, Ordering: | = help: consider using ordering modes `Acquire` or `Relaxed` instead -error: compare_exchange_weak's failure ordering may not stronger than the success ordering of `AcqRel` +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); diff --git a/tests/ui/atomic_ordering_fetch_update.stderr b/tests/ui/atomic_ordering_fetch_update.stderr index 362e104a244..694548ece97 100644 --- a/tests/ui/atomic_ordering_fetch_update.stderr +++ b/tests/ui/atomic_ordering_fetch_update.stderr @@ -79,7 +79,7 @@ LL | let _ = x.fetch_update(Ordering::SeqCst, Ordering::Release, |old| Some( | = help: consider using ordering modes `Acquire`, `SeqCst` or `Relaxed` instead -error: fetch_update's failure ordering may not stronger than the success ordering of `Release` +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)); @@ -87,7 +87,7 @@ LL | let _ = x.fetch_update(Ordering::Release, Ordering::Acquire, |old| Some | = help: consider using ordering mode `Relaxed` instead -error: fetch_update's failure ordering may not stronger than the success ordering of `Release` +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)); @@ -95,7 +95,7 @@ LL | let _ = x.fetch_update(Ordering::Release, Ordering::SeqCst, |old| Some( | = help: consider using ordering mode `Relaxed` instead -error: fetch_update's failure ordering may not stronger than the success ordering of `Relaxed` +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)); @@ -103,7 +103,7 @@ LL | let _ = x.fetch_update(Ordering::Relaxed, Ordering::SeqCst, |old| Some( | = help: consider using ordering mode `Relaxed` instead -error: fetch_update's failure ordering may not stronger than the success ordering of `Relaxed` +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)); @@ -111,7 +111,7 @@ LL | let _ = x.fetch_update(Ordering::Relaxed, Ordering::Acquire, |old| Some | = help: consider using ordering mode `Relaxed` instead -error: fetch_update's failure ordering may not stronger than the success ordering of `Acquire` +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)); @@ -119,7 +119,7 @@ LL | let _ = x.fetch_update(Ordering::Acquire, Ordering::SeqCst, |old| Some( | = help: consider using ordering modes `Acquire` or `Relaxed` instead -error: fetch_update's failure ordering may not stronger than the success ordering of `AcqRel` +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)); From 09f7a377a663043c6f63ded70436ac0969e4abc7 Mon Sep 17 00:00:00 2001 From: Thom Chiovoloni Date: Thu, 10 Sep 2020 14:50:10 -0700 Subject: [PATCH 7/7] Add comments to the invalid_atomic_ordering example --- clippy_lints/src/atomic_ordering.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/atomic_ordering.rs b/clippy_lints/src/atomic_ordering.rs index 614e33a06da..703d8a6f62b 100644 --- a/clippy_lints/src/atomic_ordering.rs +++ b/clippy_lints/src/atomic_ordering.rs @@ -22,18 +22,27 @@ declare_clippy_lint! { /// /// let x = AtomicU8::new(0); /// + /// // Bad: `Release` and `AcqRel` cannot be used for `load`. /// let _ = x.load(Ordering::Release); /// let _ = x.load(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); /// - /// let _ = x.compare_exchange(1, 2, Ordering::Relaxed, Ordering::SeqCst); - /// let _ = x.compare_exchange_weak(2, 3, Ordering::SeqCst, Ordering::Release); - /// let _ = x.fetch_update(Ordering::AcqRel, Ordering::AcqRel, |val| Some(val + val)); + /// // 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,