Skip to content

[libc++] Add -Watomic-memory-ordering diagnostic tests for atomic_ref #130206

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.

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 5 commits into from
Mar 10, 2025

Conversation

dalg24
Copy link
Member

@dalg24 dalg24 commented Mar 6, 2025

Fix #130053

@dalg24 dalg24 requested a review from a team as a code owner March 6, 2025 23:52
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Mar 6, 2025

@llvm/pr-subscribers-libcxx

Author: Damien L-G (dalg24)

Changes

Fix #130053


Full diff: https://github.com/llvm/llvm-project/pull/130206.diff

11 Files Affected:

  • (renamed) libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_strong.pass.cpp ()
  • (added) libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_strong.verify.cpp (+38)
  • (renamed) libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_weak.pass.cpp ()
  • (added) libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_weak.verify.cpp (+38)
  • (renamed) libcxx/test/libcxx/atomics/atomics.ref/ctor.pass.cpp ()
  • (renamed) libcxx/test/libcxx/atomics/atomics.ref/load.pass.cpp ()
  • (added) libcxx/test/libcxx/atomics/atomics.ref/load.verify.cpp (+33)
  • (renamed) libcxx/test/libcxx/atomics/atomics.ref/store.pass.cpp ()
  • (added) libcxx/test/libcxx/atomics/atomics.ref/store.verify.cpp (+35)
  • (renamed) libcxx/test/libcxx/atomics/atomics.ref/wait.pass.cpp ()
  • (added) libcxx/test/libcxx/atomics/atomics.ref/wait.verify.cpp (+35)
diff --git a/libcxx/test/libcxx/atomics/atomics.ref/assert.compare_exchange_strong.pass.cpp b/libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_strong.pass.cpp
similarity index 100%
rename from libcxx/test/libcxx/atomics/atomics.ref/assert.compare_exchange_strong.pass.cpp
rename to libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_strong.pass.cpp
diff --git a/libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_strong.verify.cpp b/libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_strong.verify.cpp
new file mode 100644
index 0000000000000..34ff5736c3c64
--- /dev/null
+++ b/libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_strong.verify.cpp
@@ -0,0 +1,38 @@
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: diagnose-if-support
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <atomic>
+
+// bool compare_exchange_strong(T& expected, T desired, memory_order success,
+// memory_order failure) const noexcept;
+//
+// Preconditions: failure is memory_order::relaxed, memory_order::consume,
+// memory_order::acquire, or memory_order::seq_cst.
+
+#include <atomic>
+
+void test() {
+  using T = int;
+
+  T x(T(1));
+  std::atomic_ref const a(x);
+
+  T expected(T(2));
+  T const desired(T(3));
+  std::memory_order const success = std::memory_order_relaxed;
+  // clang-format off
+  a.compare_exchange_strong(expected, desired, success, std::memory_order_relaxed);
+  a.compare_exchange_strong(expected, desired, success, std::memory_order_consume);
+  a.compare_exchange_strong(expected, desired, success, std::memory_order_acquire);
+  a.compare_exchange_strong(expected, desired, success, std::memory_order_seq_cst);
+  a.compare_exchange_strong(expected, desired, success, std::memory_order_release); // expected-warning {{memory order argument to atomic operation is invalid}}
+  a.compare_exchange_strong(expected, desired, success, std::memory_order_acq_rel); // expected-warning {{memory order argument to atomic operation is invalid}}
+  // clang-format on
+}
diff --git a/libcxx/test/libcxx/atomics/atomics.ref/assert.compare_exchange_weak.pass.cpp b/libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_weak.pass.cpp
similarity index 100%
rename from libcxx/test/libcxx/atomics/atomics.ref/assert.compare_exchange_weak.pass.cpp
rename to libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_weak.pass.cpp
diff --git a/libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_weak.verify.cpp b/libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_weak.verify.cpp
new file mode 100644
index 0000000000000..77b5a5c44d5cc
--- /dev/null
+++ b/libcxx/test/libcxx/atomics/atomics.ref/compare_exchange_weak.verify.cpp
@@ -0,0 +1,38 @@
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: diagnose-if-support
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <atomic>
+
+// bool compare_exchange_weak(T& expected, T desired, memory_order success,
+// memory_order failure) const noexcept;
+//
+// Preconditions: failure is memory_order::relaxed, memory_order::consume,
+// memory_order::acquire, or memory_order::seq_cst.
+
+#include <atomic>
+
+void test() {
+  using T = int;
+
+  T x(T(42));
+  std::atomic_ref const a(x);
+
+  T expected(T(2));
+  T const desired(T(3));
+  std::memory_order const success = std::memory_order_relaxed;
+  // clang-format off
+  a.compare_exchange_weak(expected, desired, success, std::memory_order_relaxed);
+  a.compare_exchange_weak(expected, desired, success, std::memory_order_consume);
+  a.compare_exchange_weak(expected, desired, success, std::memory_order_acquire);
+  a.compare_exchange_weak(expected, desired, success, std::memory_order_seq_cst);
+  a.compare_exchange_weak(expected, desired, success, std::memory_order_release); // expected-warning {{memory order argument to atomic operation is invalid}}
+  a.compare_exchange_weak(expected, desired, success, std::memory_order_acq_rel); // expected-warning {{memory order argument to atomic operation is invalid}}
+  // clang-format on
+}
diff --git a/libcxx/test/libcxx/atomics/atomics.ref/assert.ctor.pass.cpp b/libcxx/test/libcxx/atomics/atomics.ref/ctor.pass.cpp
similarity index 100%
rename from libcxx/test/libcxx/atomics/atomics.ref/assert.ctor.pass.cpp
rename to libcxx/test/libcxx/atomics/atomics.ref/ctor.pass.cpp
diff --git a/libcxx/test/libcxx/atomics/atomics.ref/assert.load.pass.cpp b/libcxx/test/libcxx/atomics/atomics.ref/load.pass.cpp
similarity index 100%
rename from libcxx/test/libcxx/atomics/atomics.ref/assert.load.pass.cpp
rename to libcxx/test/libcxx/atomics/atomics.ref/load.pass.cpp
diff --git a/libcxx/test/libcxx/atomics/atomics.ref/load.verify.cpp b/libcxx/test/libcxx/atomics/atomics.ref/load.verify.cpp
new file mode 100644
index 0000000000000..b2192a500833f
--- /dev/null
+++ b/libcxx/test/libcxx/atomics/atomics.ref/load.verify.cpp
@@ -0,0 +1,33 @@
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: diagnose-if-support
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <atomic>
+
+// T load(memory_order order = memory_order::seq_cst) const noexcept;
+//
+// Preconditions: order is memory_order::relaxed, memory_order::consume, memory_order::acquire, or memory_order::seq_cst.
+
+#include <atomic>
+
+void test() {
+  using T = int;
+
+  T x(T(1));
+  std::atomic_ref const a(x);
+
+  // clang-format off
+  (void)a.load(std::memory_order_relaxed);
+  (void)a.load(std::memory_order_consume);
+  (void)a.load(std::memory_order_acquire);
+  (void)a.load(std::memory_order_seq_cst);
+  (void)a.load(std::memory_order_release); // expected-warning {{memory order argument to atomic operation is invalid}}
+  (void)a.load(std::memory_order_acq_rel); // expected-warning {{memory order argument to atomic operation is invalid}}
+  // clang-format on
+}
diff --git a/libcxx/test/libcxx/atomics/atomics.ref/assert.store.pass.cpp b/libcxx/test/libcxx/atomics/atomics.ref/store.pass.cpp
similarity index 100%
rename from libcxx/test/libcxx/atomics/atomics.ref/assert.store.pass.cpp
rename to libcxx/test/libcxx/atomics/atomics.ref/store.pass.cpp
diff --git a/libcxx/test/libcxx/atomics/atomics.ref/store.verify.cpp b/libcxx/test/libcxx/atomics/atomics.ref/store.verify.cpp
new file mode 100644
index 0000000000000..ef97a3296f374
--- /dev/null
+++ b/libcxx/test/libcxx/atomics/atomics.ref/store.verify.cpp
@@ -0,0 +1,35 @@
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: diagnose-if-support
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <atomic>
+
+// void store(T desired, memory_order order = memory_order::seq_cst) const noexcept;
+//
+// Preconditions: order is memory_order::relaxed, memory_order::release, or memory_order::seq_cst.
+
+#include <atomic>
+
+void test() {
+  using T = int;
+
+  T x(T(1));
+  std::atomic_ref const a(x);
+
+  T const desired(T(2));
+
+  // clang-format off
+  a.store(desired, std::memory_order_relaxed);
+  a.store(desired, std::memory_order_release);
+  a.store(desired, std::memory_order_seq_cst);
+  a.store(desired, std::memory_order_consume); // expected-warning {{memory order argument to atomic operation is invalid}}
+  a.store(desired, std::memory_order_acquire); // expected-warning {{memory order argument to atomic operation is invalid}}
+  a.store(desired, std::memory_order_acq_rel); // expected-warning {{memory order argument to atomic operation is invalid}}
+  // clang-format on
+}
diff --git a/libcxx/test/libcxx/atomics/atomics.ref/assert.wait.pass.cpp b/libcxx/test/libcxx/atomics/atomics.ref/wait.pass.cpp
similarity index 100%
rename from libcxx/test/libcxx/atomics/atomics.ref/assert.wait.pass.cpp
rename to libcxx/test/libcxx/atomics/atomics.ref/wait.pass.cpp
diff --git a/libcxx/test/libcxx/atomics/atomics.ref/wait.verify.cpp b/libcxx/test/libcxx/atomics/atomics.ref/wait.verify.cpp
new file mode 100644
index 0000000000000..d30a55bae10f4
--- /dev/null
+++ b/libcxx/test/libcxx/atomics/atomics.ref/wait.verify.cpp
@@ -0,0 +1,35 @@
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: diagnose-if-support
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
+// <atomic>
+
+// void wait(T old, memory_order order = memory_order::seq_cst) const noexcept;
+//
+// Preconditions: order is memory_order::relaxed, memory_order::consume, memory_order::acquire, or memory_order::seq_cst.
+
+#include <atomic>
+
+void test() {
+  using T = int;
+
+  T x(T(1));
+  std::atomic_ref const a(x);
+
+  T const old(T(2));
+
+  // clang-format off
+  a.wait(old, std::memory_order_relaxed);
+  a.wait(old, std::memory_order_consume);
+  a.wait(old, std::memory_order_acquire);
+  a.wait(old, std::memory_order_seq_cst);
+  a.wait(old, std::memory_order_release); // expected-warning {{memory order argument to atomic operation is invalid}}
+  a.wait(old, std::memory_order_acq_rel); // expected-warning {{memory order argument to atomic operation is invalid}}
+  // clang-format on
+}

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix! Except for the unrelated file renames this LGTM.

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM with the complete license header.

@@ -0,0 +1,38 @@
//
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you've dropped the first line of the license header when copy/pasting.

Copy link
Member Author

@dalg24 dalg24 Mar 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are missing in the assert.* files as well. That's where I copy/pasted from.
Want me to add as part of this PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that seems like a small enough drive-by change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@dalg24 dalg24 merged commit e235432 into llvm:main Mar 10, 2025
86 checks passed
@dalg24 dalg24 deleted the atomic_ref_diagnose_memory_order branch March 10, 2025 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libc++] -Watomic-memory-ordering diagnostic tests are missing for atomic_ref
3 participants