[PATCH] D53763: [libc++] [test] Fix logic error in tests; enable for MSVC previews

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter closed this revision.
CaseyCarter added a comment.

I merged this as r351148.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53763/new/

https://reviews.llvm.org/D53763



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53763: [libc++] [test] Fix logic error in tests; enable for MSVC previews

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter added a comment.

In D53763#1356356 , @mclow.lists wrote:

> I'm a bit concerned about the `TEST_HAS_NO_SPACESHIP_OPERATOR` and how it 
> tracks with `_LIBCPP_HAS_NO_SPACESHIP_OPERATOR`, but I'm not going to hold 
> this up for that.


Same. I tried to convince the compiler guys to define a feature-test macro, but 
they're paranoid about doing so before it's in the standard. We can clean this 
up when that happens.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53763/new/

https://reviews.llvm.org/D53763



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53763: [libc++] [test] Fix logic error in tests; enable for MSVC previews

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists accepted this revision.
mclow.lists added a comment.
This revision is now accepted and ready to land.

I'm a bit concerned about the `TEST_HAS_NO_SPACESHIP_OPERATOR` and how it 
tracks with `_LIBCPP_HAS_NO_SPACESHIP_OPERATOR`, but I'm not going to hold this 
up for that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53763/new/

https://reviews.llvm.org/D53763



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53763: [libc++] [test] Fix logic error in tests; enable for MSVC previews

[Casey Carter via Phabricator via cfe-commits]

CaseyCarter created this revision.
CaseyCarter added reviewers: EricWF, mclow.lists.

Fairly straightforward: these tests were written without an implementation of 
`<=>`, and they're incorrectly testing that `0 <=> foo` has the behavior that 
is required for `foo <=> 0`.


https://reviews.llvm.org/D53763

Files:
  test/std/language.support/cmp/cmp.partialord/partialord.pass.cpp
  test/std/language.support/cmp/cmp.strongord/strongord.pass.cpp
  test/std/language.support/cmp/cmp.weakord/weakord.pass.cpp
  test/support/test_macros.h


Index: test/support/test_macros.h
===
--- test/support/test_macros.h
+++ test/support/test_macros.h
@@ -203,8 +203,9 @@
 
 // FIXME: Fix this feature check when either (A) a compiler provides a complete
 // implementation, or (b) a feature check macro is specified
+#if !defined(_MSC_VER) || defined(__clang__) || _MSC_VER < 1920 || _MSVC_LANG 
<= 201703L
 #define TEST_HAS_NO_SPACESHIP_OPERATOR
-
+#endif
 
 #if TEST_STD_VER < 11
 #define ASSERT_NOEXCEPT(...)
Index: test/std/language.support/cmp/cmp.weakord/weakord.pass.cpp
===
--- test/std/language.support/cmp/cmp.weakord/weakord.pass.cpp
+++ test/std/language.support/cmp/cmp.weakord/weakord.pass.cpp
@@ -142,7 +142,7 @@
   };
   for (auto TC : SpaceshipTestCases)
   {
-std::weak_ordering Res = (0 <=> TC.Value);
+std::weak_ordering Res = (TC.Value <=> 0);
 switch (TC.Expect) {
 case ER_Equiv:
   assert(Res == 0);
Index: test/std/language.support/cmp/cmp.strongord/strongord.pass.cpp
===
--- test/std/language.support/cmp/cmp.strongord/strongord.pass.cpp
+++ test/std/language.support/cmp/cmp.strongord/strongord.pass.cpp
@@ -185,7 +185,7 @@
   };
   for (auto TC : SpaceshipTestCases)
   {
-std::strong_ordering Res = (0 <=> TC.Value);
+std::strong_ordering Res = (TC.Value <=> 0);
 switch (TC.Expect) {
 case ER_Equiv:
   assert(Res == 0);
Index: test/std/language.support/cmp/cmp.partialord/partialord.pass.cpp
===
--- test/std/language.support/cmp/cmp.partialord/partialord.pass.cpp
+++ test/std/language.support/cmp/cmp.partialord/partialord.pass.cpp
@@ -130,7 +130,7 @@
   };
   for (auto TC : SpaceshipTestCases)
   {
-std::partial_ordering Res = (0 <=> TC.Value);
+std::partial_ordering Res = (TC.Value <=> 0);
 switch (TC.Expect) {
 case ER_Equiv:
   assert(Res == 0);


Index: test/support/test_macros.h
===
--- test/support/test_macros.h
+++ test/support/test_macros.h
@@ -203,8 +203,9 @@
 
 // FIXME: Fix this feature check when either (A) a compiler provides a complete
 // implementation, or (b) a feature check macro is specified
+#if !defined(_MSC_VER) || defined(__clang__) || _MSC_VER < 1920 || _MSVC_LANG <= 201703L
 #define TEST_HAS_NO_SPACESHIP_OPERATOR
-
+#endif
 
 #if TEST_STD_VER < 11
 #define ASSERT_NOEXCEPT(...)
Index: test/std/language.support/cmp/cmp.weakord/weakord.pass.cpp
===
--- test/std/language.support/cmp/cmp.weakord/weakord.pass.cpp
+++ test/std/language.support/cmp/cmp.weakord/weakord.pass.cpp
@@ -142,7 +142,7 @@
   };
   for (auto TC : SpaceshipTestCases)
   {
-std::weak_ordering Res = (0 <=> TC.Value);
+std::weak_ordering Res = (TC.Value <=> 0);
 switch (TC.Expect) {
 case ER_Equiv:
   assert(Res == 0);
Index: test/std/language.support/cmp/cmp.strongord/strongord.pass.cpp
===
--- test/std/language.support/cmp/cmp.strongord/strongord.pass.cpp
+++ test/std/language.support/cmp/cmp.strongord/strongord.pass.cpp
@@ -185,7 +185,7 @@
   };
   for (auto TC : SpaceshipTestCases)
   {
-std::strong_ordering Res = (0 <=> TC.Value);
+std::strong_ordering Res = (TC.Value <=> 0);
 switch (TC.Expect) {
 case ER_Equiv:
   assert(Res == 0);
Index: test/std/language.support/cmp/cmp.partialord/partialord.pass.cpp
===
--- test/std/language.support/cmp/cmp.partialord/partialord.pass.cpp
+++ test/std/language.support/cmp/cmp.partialord/partialord.pass.cpp
@@ -130,7 +130,7 @@
   };
   for (auto TC : SpaceshipTestCases)
   {
-std::partial_ordering Res = (0 <=> TC.Value);
+std::partial_ordering Res = (TC.Value <=> 0);
 switch (TC.Expect) {
 case ER_Equiv:
   assert(Res == 0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits