On Jul 22, 2016 4:45 PM, "Eric Fiselier" wrote:
>
> EricWF added inline comments.
>
>
> Comment at: test/libcxx/atomics/diagnose_invalid_memory_order.fail.cpp:87
> @@ +86,3 @@
> +x.compare_exchange_weak(val1, val2, std::memory_order_release);
> +}
> +{
> --
EricWF added inline comments.
Comment at: test/libcxx/atomics/diagnose_invalid_memory_order.fail.cpp:87
@@ +86,3 @@
+x.compare_exchange_weak(val1, val2, std::memory_order_release);
+}
+{
jfb wrote:
> That's not quite true: the failure ordering is a
jfb added inline comments.
Comment at: test/libcxx/atomics/diagnose_invalid_memory_order.fail.cpp:87
@@ +86,3 @@
+x.compare_exchange_weak(val1, val2, std::memory_order_release);
+}
+{
That's not quite true: the failure ordering is auto-deduced from
EricWF updated this revision to Diff 65140.
EricWF added a comment.
Add comment about checking "stronger" orderings.
https://reviews.llvm.org/D22557
Files:
include/atomic
test/libcxx/atomics/diagnose_invalid_memory_order.fail.cpp
Index: test/libcxx/atomics/diagnose_invalid_memory_order.fai
EricWF added a reviewer: jfb.
EricWF marked an inline comment as done.
EricWF added a comment.
Address inline comments from @jfb and add him as a reviewer.
Comment at: test/libcxx/atomics/diagnose_invalid_memory_order.fail.cpp:86
@@ +85,3 @@
+// does not generate any dia
jfb added a comment.
Two comments, then lgtm.
Comment at: include/atomic:581
@@ +580,3 @@
+ || __f == memory_order_acq_rel, ""))) \
+__attribute__ ((__unavailable__("memory order argument to atomic operation
is invalid")))
+#endif
bcraig added inline comments.
Comment at: include/atomic:569
@@ +568,3 @@
+__attribute__ ((__enable_if__(__m == memory_order_release \
+ || __m == memory_order_acq_rel, ""))) \
+__attribute__ ((__unavailable__("memory
EricWF updated this revision to Diff 64808.
EricWF added a comment.
Add tests for valid memory orderings as well. Originally I figured the rest of
the test suite should cover them but there's no harm in more tests.
https://reviews.llvm.org/D22557
Files:
include/atomic
test/libcxx/atomics/d
EricWF marked 2 inline comments as done.
Comment at: include/atomic:581
@@ +580,3 @@
+ || __f == memory_order_acq_rel, ""))) \
+__attribute__ ((__unavailable__("memory order argument to atomic operation
is invalid")))
+#endif
---
jfb added inline comments.
Comment at: include/atomic:569
@@ +568,3 @@
+__attribute__ ((__enable_if__(__m == memory_order_release \
+ || __m == memory_order_acq_rel, ""))) \
+__attribute__ ((__unavailable__("memory ord
bcraig added a subscriber: bcraig.
Comment at: include/atomic:569
@@ +568,3 @@
+__attribute__ ((__enable_if__(__m == memory_order_release \
+ || __m == memory_order_acq_rel, ""))) \
+__attribute__ ((__unavailable__("me
jfb added a subscriber: jfb.
jfb added a comment.
Awesome, thanks for doing this!
Should this be a warning or an error?
Comment at: include/atomic:581
@@ +580,3 @@
+ || __f == memory_order_acq_rel, ""))) \
+__attribute__ ((__unavailabl
EricWF created this revision.
EricWF added reviewers: mclow.lists, rsmith.
EricWF added subscribers: cfe-commits, rsmith.
This patch uses the __attribute__((enable_if)) hack suggested by @rsmith to
diagnose invalid arguments when possible.
In order to diagnose an invalid argument `m` to `f(m)` w
13 matches
Mail list logo
