Skip to content

Commit

Permalink
Rollup merge of rust-lang#99410 - tmiasko:atomic-lint, r=fee1-dead
Browse files Browse the repository at this point in the history 
Update invalid atomic ordering lint

The restriction that success ordering must be at least as strong as its
failure ordering in compare-exchange operations was lifted in rust-lang#98383.
  • Loading branch information
@matthiaskrgr
matthiaskrgr authored Jul 18, 2022
2 parents c3a38b8 + 45afc21 commit 8fc0b8d
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 297 deletions.
5 changes: 0 additions & 5 deletions compiler/rustc_error_messages/locales/en-US/lint.ftl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,6 @@ lint-atomic-ordering-invalid = `{$method}`'s failure ordering may not be `Releas
.label = invalid failure ordering
.help = consider using `Acquire` or `Relaxed` failure ordering instead
lint-atomic-ordering-invalid-fail-success = `{$method}`'s success ordering must be at least as strong as its failure ordering
.fail-label = `{$fail_ordering}` failure ordering
.success-label = `{$success_ordering}` success ordering
.suggestion = consider using `{$success_suggestion}` success ordering instead
lint-unused-op = unused {$op} that must be used
.label = the {$op} produces a value
.suggestion = use `let _ = ...` to ignore the resulting value
Expand Down
41 changes: 3 additions & 38 deletions compiler/rustc_lint/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1434,10 +1434,6 @@ declare_lint! {
/// - Passing `Ordering::Release` or `Ordering::AcqRel` as the failure
/// ordering for any of `AtomicType::compare_exchange`,
/// `AtomicType::compare_exchange_weak`, or `AtomicType::fetch_update`.
///
/// - Passing in a pair of orderings to `AtomicType::compare_exchange`,
/// `AtomicType::compare_exchange_weak`, or `AtomicType::fetch_update`
/// where the failure ordering is stronger than the success ordering.
INVALID_ATOMIC_ORDERING,
Deny,
"usage of invalid atomic ordering in atomic operations and memory fences"
Expand Down Expand Up @@ -1544,9 +1540,9 @@ impl InvalidAtomicOrdering {
let Some((method, args)) = Self::inherent_atomic_method_call(cx, expr, &[sym::fetch_update, sym::compare_exchange, sym::compare_exchange_weak])
else {return };

let (success_order_arg, fail_order_arg) = match method {
sym::fetch_update => (&args[1], &args[2]),
sym::compare_exchange | sym::compare_exchange_weak => (&args[3], &args[4]),
let fail_order_arg = match method {
sym::fetch_update => &args[2],
sym::compare_exchange | sym::compare_exchange_weak => &args[4],
_ => return,
};

Expand All @@ -1568,37 +1564,6 @@ impl InvalidAtomicOrdering {
InvalidAtomicOrderingDiag { method, fail_order_arg_span: fail_order_arg.span },
);
}

let Some(success_ordering) = Self::match_ordering(cx, success_order_arg) else { return };

if matches!(
(success_ordering, fail_ordering),
(sym::Relaxed | sym::Release, sym::Acquire)
| (sym::Relaxed | sym::Release | sym::Acquire | sym::AcqRel, sym::SeqCst)
) {
let success_suggestion =
if success_ordering == sym::Release && fail_ordering == sym::Acquire {
sym::AcqRel
} else {
fail_ordering
};
cx.struct_span_lint(INVALID_ATOMIC_ORDERING, success_order_arg.span, |diag| {
diag.build(fluent::lint::atomic_ordering_invalid_fail_success)
.set_arg("method", method)
.set_arg("fail_ordering", fail_ordering)
.set_arg("success_ordering", success_ordering)
.set_arg("success_suggestion", success_suggestion)
.span_label(fail_order_arg.span, fluent::lint::fail_label)
.span_label(success_order_arg.span, fluent::lint::success_label)
.span_suggestion_short(
success_order_arg.span,
fluent::lint::suggestion,
format!("std::sync::atomic::Ordering::{success_suggestion}"),
Applicability::MaybeIncorrect,
)
.emit();
});
}
}
}

Expand Down
28 changes: 8 additions & 20 deletions src/test/ui/lint/lint-invalid-atomic-ordering-exchange-weak.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,17 @@ fn main() {

// 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::Relaxed, Ordering::Acquire);
let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::SeqCst);
let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Acquire, Ordering::Relaxed);
let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Acquire, Ordering::Acquire);
let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Acquire, Ordering::SeqCst);
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(ptr2, ptr, Ordering::Release, Ordering::Acquire);
let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::SeqCst);
let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::AcqRel, Ordering::Relaxed);
let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::AcqRel, Ordering::Acquire);
let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::AcqRel, Ordering::SeqCst);
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);
Expand Down Expand Up @@ -41,22 +47,4 @@ fn main() {
//~^ ERROR `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`
let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::SeqCst, Ordering::Release);
//~^ ERROR `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`

// Release success order forbids failure order of Acquire or SeqCst
let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::Acquire);
//~^ ERROR `compare_exchange_weak`'s success ordering must be at least as strong as
let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::SeqCst);
//~^ ERROR `compare_exchange_weak`'s success ordering must be at least as strong as

// Relaxed success order also forbids failure order of Acquire or SeqCst
let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::SeqCst);
//~^ ERROR `compare_exchange_weak`'s success ordering must be at least as strong as
let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::Acquire);
//~^ ERROR `compare_exchange_weak`'s success ordering must be at least as strong as

// Acquire/AcqRel forbids failure order of SeqCst
let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Acquire, Ordering::SeqCst);
//~^ ERROR `compare_exchange_weak`'s success ordering must be at least as strong as
let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::AcqRel, Ordering::SeqCst);
//~^ ERROR `compare_exchange_weak`'s success ordering must be at least as strong as
}
76 changes: 11 additions & 65 deletions src/test/ui/lint/lint-invalid-atomic-ordering-exchange-weak.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`, since a failed `compare_exchange_weak` does not result in a write
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:22:67
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:28:67
|
LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Relaxed, Ordering::AcqRel);
| ^^^^^^^^^^^^^^^^ invalid failure ordering
Expand All @@ -8,130 +8,76 @@ LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Relaxed, Ordering:
= help: consider using `Acquire` or `Relaxed` failure ordering instead

error: `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`, since a failed `compare_exchange_weak` does not result in a write
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:24:67
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:30:67
|
LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Acquire, Ordering::AcqRel);
| ^^^^^^^^^^^^^^^^ invalid failure ordering
|
= help: consider using `Acquire` or `Relaxed` failure ordering instead

error: `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`, since a failed `compare_exchange_weak` does not result in a write
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:26:67
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:32:67
|
LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::AcqRel);
| ^^^^^^^^^^^^^^^^ invalid failure ordering
|
= help: consider using `Acquire` or `Relaxed` failure ordering instead

error: `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`, since a failed `compare_exchange_weak` does not result in a write
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:28:66
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:34:66
|
LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::AcqRel, Ordering::AcqRel);
| ^^^^^^^^^^^^^^^^ invalid failure ordering
|
= help: consider using `Acquire` or `Relaxed` failure ordering instead

error: `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`, since a failed `compare_exchange_weak` does not result in a write
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:30:66
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:36:66
|
LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::SeqCst, Ordering::AcqRel);
| ^^^^^^^^^^^^^^^^ invalid failure ordering
|
= help: consider using `Acquire` or `Relaxed` failure ordering instead

error: `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`, since a failed `compare_exchange_weak` does not result in a write
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:34:67
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:40:67
|
LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::Release);
| ^^^^^^^^^^^^^^^^^ invalid failure ordering
|
= help: consider using `Acquire` or `Relaxed` failure ordering instead

error: `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`, since a failed `compare_exchange_weak` does not result in a write
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:36:67
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:42:67
|
LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Acquire, Ordering::Release);
| ^^^^^^^^^^^^^^^^^ invalid failure ordering
|
= help: consider using `Acquire` or `Relaxed` failure ordering instead

error: `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`, since a failed `compare_exchange_weak` does not result in a write
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:38:67
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:44:67
|
LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Release, Ordering::Release);
| ^^^^^^^^^^^^^^^^^ invalid failure ordering
|
= help: consider using `Acquire` or `Relaxed` failure ordering instead

error: `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`, since a failed `compare_exchange_weak` does not result in a write
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:40:66
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:46:66
|
LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::AcqRel, Ordering::Release);
| ^^^^^^^^^^^^^^^^^ invalid failure ordering
|
= help: consider using `Acquire` or `Relaxed` failure ordering instead

error: `compare_exchange_weak`'s failure ordering may not be `Release` or `AcqRel`, since a failed `compare_exchange_weak` does not result in a write
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:42:66
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:48:66
|
LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::SeqCst, Ordering::Release);
| ^^^^^^^^^^^^^^^^^ invalid failure ordering
|
= help: consider using `Acquire` or `Relaxed` failure ordering instead

error: `compare_exchange_weak`'s success ordering must be at least as strong as its failure ordering
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:46:48
|
LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::Acquire);
| ^^^^^^^^^^^^^^^^^ ----------------- `Acquire` failure ordering
| |
| `Release` success ordering
| help: consider using `AcqRel` success ordering instead

error: `compare_exchange_weak`'s success ordering must be at least as strong as its failure ordering
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:48:48
|
LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Release, Ordering::SeqCst);
| ^^^^^^^^^^^^^^^^^ ---------------- `SeqCst` failure ordering
| |
| `Release` success ordering
| help: consider using `SeqCst` success ordering instead

error: `compare_exchange_weak`'s success ordering must be at least as strong as its failure ordering
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:52:48
|
LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::SeqCst);
| ^^^^^^^^^^^^^^^^^ ---------------- `SeqCst` failure ordering
| |
| `Relaxed` success ordering
| help: consider using `SeqCst` success ordering instead

error: `compare_exchange_weak`'s success ordering must be at least as strong as its failure ordering
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:54:48
|
LL | let _ = x.compare_exchange_weak(ptr, ptr2, Ordering::Relaxed, Ordering::Acquire);
| ^^^^^^^^^^^^^^^^^ ----------------- `Acquire` failure ordering
| |
| `Relaxed` success ordering
| help: consider using `Acquire` success ordering instead

error: `compare_exchange_weak`'s success ordering must be at least as strong as its failure ordering
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:58:48
|
LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::Acquire, Ordering::SeqCst);
| ^^^^^^^^^^^^^^^^^ ---------------- `SeqCst` failure ordering
| |
| `Acquire` success ordering
| help: consider using `SeqCst` success ordering instead

error: `compare_exchange_weak`'s success ordering must be at least as strong as its failure ordering
--> $DIR/lint-invalid-atomic-ordering-exchange-weak.rs:60:48
|
LL | let _ = x.compare_exchange_weak(ptr2, ptr, Ordering::AcqRel, Ordering::SeqCst);
| ^^^^^^^^^^^^^^^^ ---------------- `SeqCst` failure ordering
| |
| `AcqRel` success ordering
| help: consider using `SeqCst` success ordering instead

error: aborting due to 16 previous errors
error: aborting due to 10 previous errors

28 changes: 8 additions & 20 deletions src/test/ui/lint/lint-invalid-atomic-ordering-exchange.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,17 @@ fn main() {

// 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::Relaxed, Ordering::Acquire);
let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::SeqCst);
let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::Relaxed);
let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::Acquire);
let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::SeqCst);
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::Release, Ordering::Acquire);
let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::SeqCst);
let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::Relaxed);
let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::Acquire);
let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::SeqCst);
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);
Expand Down Expand Up @@ -39,22 +45,4 @@ fn main() {
//~^ ERROR `compare_exchange`'s failure ordering may not be `Release` or `AcqRel`
let _ = x.compare_exchange(0, 0, Ordering::SeqCst, Ordering::Release);
//~^ ERROR `compare_exchange`'s failure ordering may not be `Release` or `AcqRel`

// Release success order forbids failure order of Acquire or SeqCst
let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::Acquire);
//~^ ERROR `compare_exchange`'s success ordering must be at least as strong as
let _ = x.compare_exchange(0, 0, Ordering::Release, Ordering::SeqCst);
//~^ ERROR `compare_exchange`'s success ordering must be at least as strong as

// Relaxed success order also forbids failure order of Acquire or SeqCst
let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::SeqCst);
//~^ ERROR `compare_exchange`'s success ordering must be at least as strong as
let _ = x.compare_exchange(0, 0, Ordering::Relaxed, Ordering::Acquire);
//~^ ERROR `compare_exchange`'s success ordering must be at least as strong as

// Acquire/AcqRel forbids failure order of SeqCst
let _ = x.compare_exchange(0, 0, Ordering::Acquire, Ordering::SeqCst);
//~^ ERROR `compare_exchange`'s success ordering must be at least as strong as
let _ = x.compare_exchange(0, 0, Ordering::AcqRel, Ordering::SeqCst);
//~^ ERROR `compare_exchange`'s success ordering must be at least as strong as
}
Loading

0 comments on commit 8fc0b8d

Please sign in to comment.