Skip to content

Update invalid atomic ordering lint #99410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 19, 2022
Merged

Update invalid atomic ordering lint #99410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions compiler/rustc_error_messages/locales/en-US/lint.ftl
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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