You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/09/04 19:21:57 UTC
[GitHub] [arrow] xhochy opened a new pull request #8114: ARROW-9588: [C++] Partially support building with clang in an MSVC setting
xhochy opened a new pull request #8114:
URL: https://github.com/apache/arrow/pull/8114
This is only half the work, thus we don't have a test for this yet but I wanted to upstream this to have a local diff less lying around.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #8114: ARROW-9588: [C++] Partially support building with clang in an MSVC setting
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8114:
URL: https://github.com/apache/arrow/pull/8114#discussion_r484472463
##########
File path: cpp/src/arrow/python/type_traits.h
##########
@@ -105,7 +105,7 @@ struct npy_traits<NPY_FLOAT32> {
using TypeClass = FloatType;
using BuilderClass = FloatBuilder;
- static constexpr float na_sentinel = NAN;
+ static constexpr float na_sentinel = std::numeric_limits<float>::quiet_NaN();
Review comment:
Can you explain why this change is necessary? The `NAN` macro is standard: https://en.cppreference.com/w/cpp/numeric/math/NAN
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou closed pull request #8114: ARROW-9588: [C++] Partially support building with clang in an MSVC setting
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #8114:
URL: https://github.com/apache/arrow/pull/8114
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] xhochy commented on a change in pull request #8114: ARROW-9588: [C++] Partially support building with clang in an MSVC setting
Posted by GitBox <gi...@apache.org>.
xhochy commented on a change in pull request #8114:
URL: https://github.com/apache/arrow/pull/8114#discussion_r484801005
##########
File path: cpp/src/arrow/python/type_traits.h
##########
@@ -105,7 +105,7 @@ struct npy_traits<NPY_FLOAT32> {
using TypeClass = FloatType;
using BuilderClass = FloatBuilder;
- static constexpr float na_sentinel = NAN;
+ static constexpr float na_sentinel = std::numeric_limits<float>::quiet_NaN();
Review comment:
Added a comment, the issue is that the Windows implementation of the macro is not fully compatible with clang's stictness of `constexpr`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8114: ARROW-9588: [C++] Partially support building with clang in an MSVC setting
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8114:
URL: https://github.com/apache/arrow/pull/8114#issuecomment-687398603
It seems that `@github-actions autotune` doesn't support fixing CMake format yet: https://github.com/apache/arrow/blob/master/.github/workflows/comment_bot.yml#L87
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] xhochy commented on a change in pull request #8114: ARROW-9588: [C++] Partially support building with clang in an MSVC setting
Posted by GitBox <gi...@apache.org>.
xhochy commented on a change in pull request #8114:
URL: https://github.com/apache/arrow/pull/8114#discussion_r484801005
##########
File path: cpp/src/arrow/python/type_traits.h
##########
@@ -105,7 +105,7 @@ struct npy_traits<NPY_FLOAT32> {
using TypeClass = FloatType;
using BuilderClass = FloatBuilder;
- static constexpr float na_sentinel = NAN;
+ static constexpr float na_sentinel = std::numeric_limits<float>::quiet_NaN();
Review comment:
Added a comment, the issue is that the Windows implementation of the macro is not fully compatible with clang's stictness.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] xhochy commented on pull request #8114: ARROW-9588: [C++] Partially support building with clang in an MSVC setting
Posted by GitBox <gi...@apache.org>.
xhochy commented on pull request #8114:
URL: https://github.com/apache/arrow/pull/8114#issuecomment-687338515
I remember seeing that we can run the formatting fixes via a @github-actions command. Can someone enlighten me what is was?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8114: ARROW-9588: [C++] Partially support building with clang in an MSVC setting
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8114:
URL: https://github.com/apache/arrow/pull/8114#issuecomment-687340021
https://issues.apache.org/jira/browse/ARROW-9588
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #8114: ARROW-9588: [C++] Partially support building with clang in an MSVC setting
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8114:
URL: https://github.com/apache/arrow/pull/8114#discussion_r484805309
##########
File path: cpp/src/arrow/python/type_traits.h
##########
@@ -105,7 +105,7 @@ struct npy_traits<NPY_FLOAT32> {
using TypeClass = FloatType;
using BuilderClass = FloatBuilder;
- static constexpr float na_sentinel = NAN;
+ static constexpr float na_sentinel = std::numeric_limits<float>::quiet_NaN();
Review comment:
Thank you. Just: "division" (typo)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org