-
Notifications
You must be signed in to change notification settings - Fork 69
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
Replace spelled out logical operators: breaks Kokkos windows tests #344
Conversation
c3c32bc
to
467e567
Compare
Alternatively, you could do diff --git a/include/experimental/__p0009_bits/macros.hpp b/include/experimental/__p0009_bits/macros.hpp
index 30209a6..34e2528 100644
--- a/include/experimental/__p0009_bits/macros.hpp
+++ b/include/experimental/__p0009_bits/macros.hpp
@@ -25,6 +25,10 @@
#include "assert.h"
#endif
+#ifdef _MSC_VER
+#include <iso646.h>
+#endif
+
#ifndef _MDSPAN_HOST_DEVICE
# if defined(_MDSPAN_HAS_CUDA) || defined(_MDSPAN_HAS_HIP)
# define _MDSPAN_HOST_DEVICE __host__ __device__ |
Let's avoid forcing default behavior change on downstream users. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is this clang-tidy check https://clang.llvm.org/extra/clang-tidy/checks/readability/operators-representation.html if this can help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all of them look correct to me
Co-authored-by: Thomas Padioleau <[email protected]>
I was going to say that including https://learn.microsoft.com/en-us/cpp/c-runtime-library/iso646-operators?view=msvc-170 The alternative spellings are required by the C++ Standard. Windows is nonconforming because it requires use of a header. That being said, I don't object to the proposed solution. |
Including that thing is transitive though, do we really want to change the default behavior of MSVC for everyone downstream? |
Just FYI, including that header doesn't "change the default behavior of MSVC," it makes MSVC conform to the C++ Standard a bit better. That being said, I don't object to the proposed solution. |
Errors such as: