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 2022/02/02 00:19:10 UTC
[GitHub] [arrow] StephanTLavavej opened a new pull request #12317: ARROW-15520: Qualify `arrow_vendored::date::format()` for C++20 compatibility
StephanTLavavej opened a new pull request #12317:
URL: https://github.com/apache/arrow/pull/12317
As explained in ARROW-15520, these unqualified calls to `format()` are ambiguous in the C++20 Standard.
The `using`-declaration `using arrow_vendored::date::format;` makes the compiler consider the desired overload, but it doesn't automatically win. Argument-Dependent Lookup also considers `std::format()` because the arguments are `std::chrono::duration` types (and `<chrono>` includes `<format>` in MSVC's implementation). A very recent change to `std::format()`'s signature in a C++20 Defect Report makes it an equally good match as the desired `arrow_vendored::date::format()` overload, so the compiler emits an ambiguity error.
The fix is simple, although slightly verbose - the code simply needs to explicitly qualify each call, in order to defend against Argument-Dependent Lookup. The fix is also perfectly backwards-compatible (i.e. it works in previous Standard versions, and with all other platforms).
(Also as mentioned in ARROW-15520, although this requires building Apache Arrow with non-default settings to use the latest C++ Standard version, this change is good for future-proofing and will make it easier for the MSVC team to continue validation that prevents toolset regressions that could affect Apache Arrow and other projects.)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou closed pull request #12317: ARROW-15520: [C++] Qualify `arrow_vendored::date::format()` for C++20 compatibility
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #12317:
URL: https://github.com/apache/arrow/pull/12317
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #12317: ARROW-15520: [C++] Qualify `arrow_vendored::date::format()` for C++20 compatibility
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12317:
URL: https://github.com/apache/arrow/pull/12317#issuecomment-1028038961
Benchmark runs are scheduled for baseline = 56386a450dd3216066ee4230789acd706e574fe5 and contender = fb5a4f6c4158178608be475a07ddeaab59fff149. fb5a4f6c4158178608be475a07ddeaab59fff149 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/fbf8eeec483b4a81a8096885a153b1a4...1ab735284d49440481c73833f73e2dd3/)
[Finished :arrow_down:0.36% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/7195b1f6a57d4dfaa7fe62212ddb86fe...f22323f3ba0a4ca484923019f0e6f779/)
[Finished :arrow_down:0.26% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/6660284e906e4452bd197d07d467061e...65d9d2a7c22a422788f9a6d8bab001aa/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] StephanTLavavej commented on pull request #12317: ARROW-15520: [C++] Qualify `arrow_vendored::date::format()` for C++20 compatibility
Posted by GitBox <gi...@apache.org>.
StephanTLavavej commented on pull request #12317:
URL: https://github.com/apache/arrow/pull/12317#issuecomment-1027814778
Although I could have accepted clang-format's wrapping, it seemed ugly (since it consumes extra lines and makes the code not line up nicely). After seeing that namespace aliases are used elsewhere in the codebase, I introduced one here. For consistency, I changed an extra line to use the new alias, considerably shortening it.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #12317: ARROW-15520: [C++] Qualify `arrow_vendored::date::format()` for C++20 compatibility
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #12317:
URL: https://github.com/apache/arrow/pull/12317#issuecomment-1028038961
Benchmark runs are scheduled for baseline = 56386a450dd3216066ee4230789acd706e574fe5 and contender = fb5a4f6c4158178608be475a07ddeaab59fff149. fb5a4f6c4158178608be475a07ddeaab59fff149 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/fbf8eeec483b4a81a8096885a153b1a4...1ab735284d49440481c73833f73e2dd3/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/7195b1f6a57d4dfaa7fe62212ddb86fe...f22323f3ba0a4ca484923019f0e6f779/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/6660284e906e4452bd197d07d467061e...65d9d2a7c22a422788f9a6d8bab001aa/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #12317: ARROW-15520: [C++] Qualify `arrow_vendored::date::format()` for C++20 compatibility
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #12317:
URL: https://github.com/apache/arrow/pull/12317#discussion_r797598878
##########
File path: cpp/src/arrow/array/diff.cc
##########
@@ -639,42 +639,42 @@ class MakeFormatterImpl {
auto fmt = fmt_str.c_str();
auto unit = checked_cast<const T&>(*array.type()).unit();
auto value = checked_cast<const NumericArray<T>&>(array).Value(index);
- using arrow_vendored::date::format;
+ namespace avd = arrow_vendored::date;
Review comment:
```suggestion
// Using unqualified `format` directly would produce ambiguous
// lookup because of `std::format` (ARROW-15520).
namespace avd = arrow_vendored::date;
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #12317: ARROW-15520: [C++] Qualify `arrow_vendored::date::format()` for C++20 compatibility
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12317:
URL: https://github.com/apache/arrow/pull/12317#issuecomment-1028038961
Benchmark runs are scheduled for baseline = 56386a450dd3216066ee4230789acd706e574fe5 and contender = fb5a4f6c4158178608be475a07ddeaab59fff149. fb5a4f6c4158178608be475a07ddeaab59fff149 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/fbf8eeec483b4a81a8096885a153b1a4...1ab735284d49440481c73833f73e2dd3/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/7195b1f6a57d4dfaa7fe62212ddb86fe...f22323f3ba0a4ca484923019f0e6f779/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/6660284e906e4452bd197d07d467061e...65d9d2a7c22a422788f9a6d8bab001aa/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #12317: ARROW-15520: Qualify `arrow_vendored::date::format()` for C++20 compatibility
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12317:
URL: https://github.com/apache/arrow/pull/12317#issuecomment-1027423509
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] StephanTLavavej commented on pull request #12317: ARROW-15520: [C++] Qualify `arrow_vendored::date::format()` for C++20 compatibility
Posted by GitBox <gi...@apache.org>.
StephanTLavavej commented on pull request #12317:
URL: https://github.com/apache/arrow/pull/12317#issuecomment-1027809220
Aha, clang-format has rejected overly long lines. (The microsoft/STL repo is also a huge fan of clang-format!) I'll validate and push a commit to fix this.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #12317: ARROW-15520: [C++] Qualify `arrow_vendored::date::format()` for C++20 compatibility
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12317:
URL: https://github.com/apache/arrow/pull/12317#issuecomment-1028038961
Benchmark runs are scheduled for baseline = 56386a450dd3216066ee4230789acd706e574fe5 and contender = fb5a4f6c4158178608be475a07ddeaab59fff149. fb5a4f6c4158178608be475a07ddeaab59fff149 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/fbf8eeec483b4a81a8096885a153b1a4...1ab735284d49440481c73833f73e2dd3/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/7195b1f6a57d4dfaa7fe62212ddb86fe...f22323f3ba0a4ca484923019f0e6f779/)
[Finished :arrow_down:0.26% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/6660284e906e4452bd197d07d467061e...65d9d2a7c22a422788f9a6d8bab001aa/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org