pitrou commented on a change in pull request #10583:
URL: https://github.com/apache/arrow/pull/10583#discussion_r659900556
##########
File path: dev/tasks/vcpkg-tests/github.windows.yml
##########
@@ -36,6 +36,7 @@ jobs:
git -C arrow checkout FETCH_HEAD
git -C arrow submodule update --init --recursive
- name: Remove and Reinstall vcpkg
+ # TODO: this comment seems outdated, the vcpkg on GHA is quite recent.
Review comment:
Note @kou @kszucs : I tried to remove the vcpkg-reinstall step, but then
got an error:
```
Error: Couldn't find explicitly specified baseline
`"a267ab118c09f56f3dae96c9a4b3410820ad2f0b"` in the baseline file, and there
was no baseline at that commit or the commit didn't exist.
Error: while checking out baseline
'a267ab118c09f56f3dae96c9a4b3410820ad2f0b:versions/baseline.json':
fatal: path 'versions/baseline.json' exists on disk, but not in
'a267ab118c09f56f3dae96c9a4b3410820ad2f0b'
```
https://github.com/ursacomputing/crossbow/runs/2933445520?check_suite_focus=true
##########
File path: cpp/src/parquet/statistics.cc
##########
@@ -83,12 +87,25 @@ struct UnsignedCompareHelperBase {
using T = typename DType::c_type;
using UCType = typename std::make_unsigned<T>::type;
- constexpr static T DefaultMin() { return std::numeric_limits<UCType>::max();
}
- constexpr static T DefaultMax() { return
std::numeric_limits<UCType>::lowest(); }
+ static_assert(!std::is_same<T, UCType>::value, "T is unsigned");
+ static_assert(sizeof(T) == sizeof(UCType), "T and UCType not the same size");
+
+ // NOTE: according to the C++ spec, unsigned-to-signed conversion is
+ // implementation-defined if the original value does not fix in the signed
type
+ // (i.e., two's complement cannot be assumed even on mainstream machines,
+ // because the compiler may decide otherwise). Hence the use of `SafeCopy`
+ // below for deterministic bit-casting.
+ // (see "Integer conversions" in
+ // https://en.cppreference.com/w/cpp/language/implicit_conversion)
+
+ static const T DefaultMin() { return
SafeCopy<T>(std::numeric_limits<UCType>::max()); }
+ static const T DefaultMax() {
+ return SafeCopy<T>(std::numeric_limits<UCType>::lowest());
Review comment:
Entirely right. I'll do.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
