pr12317-qualify-format.patch 4.4 KB

1234567891011121314151617181920212223242526272829303132333435363738394041424344454647484950515253545556575859606162636465666768697071727374757677787980818283
  1. From fb5a4f6c4158178608be475a07ddeaab59fff149 Mon Sep 17 00:00:00 2001
  2. From: "Stephan T. Lavavej" <stl@microsoft.com>
  3. Date: Wed, 2 Feb 2022 16:04:24 +0100
  4. Subject: [PATCH] ARROW-15520: [C++] Qualify `arrow_vendored::date::format()`
  5. for C++20 compatibility
  6. As explained in ARROW-15520, these unqualified calls to `format()` are ambiguous in the C++20 Standard.
  7. 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.
  8. 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).
  9. (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.)
  10. Closes #12317 from StephanTLavavej/cxx20-format
  11. Lead-authored-by: Stephan T. Lavavej <stl@microsoft.com>
  12. Co-authored-by: Antoine Pitrou <pitrou@free.fr>
  13. Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
  14. Signed-off-by: Antoine Pitrou <antoine@python.org>
  15. ---
  16. cpp/src/arrow/array/diff.cc | 22 ++++++++++++----------
  17. 1 file changed, 12 insertions(+), 10 deletions(-)
  18. diff --git a/cpp/src/arrow/array/diff.cc b/cpp/src/arrow/array/diff.cc
  19. index bcc065c69b42c..16f4f9c7638a8 100644
  20. --- a/cpp/src/arrow/array/diff.cc
  21. +++ b/cpp/src/arrow/array/diff.cc
  22. @@ -639,42 +639,44 @@ class MakeFormatterImpl {
  23. auto fmt = fmt_str.c_str();
  24. auto unit = checked_cast<const T&>(*array.type()).unit();
  25. auto value = checked_cast<const NumericArray<T>&>(array).Value(index);
  26. - using arrow_vendored::date::format;
  27. + // Using unqualified `format` directly would produce ambiguous
  28. + // lookup because of `std::format` (ARROW-15520).
  29. + namespace avd = arrow_vendored::date;
  30. using std::chrono::nanoseconds;
  31. using std::chrono::microseconds;
  32. using std::chrono::milliseconds;
  33. using std::chrono::seconds;
  34. if (AddEpoch) {
  35. - static arrow_vendored::date::sys_days epoch{arrow_vendored::date::jan / 1 / 1970};
  36. + static avd::sys_days epoch{avd::jan / 1 / 1970};
  37. switch (unit) {
  38. case TimeUnit::NANO:
  39. - *os << format(fmt, static_cast<nanoseconds>(value) + epoch);
  40. + *os << avd::format(fmt, static_cast<nanoseconds>(value) + epoch);
  41. break;
  42. case TimeUnit::MICRO:
  43. - *os << format(fmt, static_cast<microseconds>(value) + epoch);
  44. + *os << avd::format(fmt, static_cast<microseconds>(value) + epoch);
  45. break;
  46. case TimeUnit::MILLI:
  47. - *os << format(fmt, static_cast<milliseconds>(value) + epoch);
  48. + *os << avd::format(fmt, static_cast<milliseconds>(value) + epoch);
  49. break;
  50. case TimeUnit::SECOND:
  51. - *os << format(fmt, static_cast<seconds>(value) + epoch);
  52. + *os << avd::format(fmt, static_cast<seconds>(value) + epoch);
  53. break;
  54. }
  55. return;
  56. }
  57. switch (unit) {
  58. case TimeUnit::NANO:
  59. - *os << format(fmt, static_cast<nanoseconds>(value));
  60. + *os << avd::format(fmt, static_cast<nanoseconds>(value));
  61. break;
  62. case TimeUnit::MICRO:
  63. - *os << format(fmt, static_cast<microseconds>(value));
  64. + *os << avd::format(fmt, static_cast<microseconds>(value));
  65. break;
  66. case TimeUnit::MILLI:
  67. - *os << format(fmt, static_cast<milliseconds>(value));
  68. + *os << avd::format(fmt, static_cast<milliseconds>(value));
  69. break;
  70. case TimeUnit::SECOND:
  71. - *os << format(fmt, static_cast<seconds>(value));
  72. + *os << avd::format(fmt, static_cast<seconds>(value));
  73. break;
  74. }
  75. };