From fb5a4f6c4158178608be475a07ddeaab59fff149 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 2 Feb 2022 16:04:24 +0100 Subject: [PATCH] ARROW-15520: [C++] Qualify `arrow_vendored::date::format()` for C++20 compatibility 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 `` includes `` 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.) Closes #12317 from StephanTLavavej/cxx20-format Lead-authored-by: Stephan T. Lavavej Co-authored-by: Antoine Pitrou Co-authored-by: Stephan T. Lavavej Signed-off-by: Antoine Pitrou --- cpp/src/arrow/array/diff.cc | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/cpp/src/arrow/array/diff.cc b/cpp/src/arrow/array/diff.cc index bcc065c69b42c..16f4f9c7638a8 100644 --- a/cpp/src/arrow/array/diff.cc +++ b/cpp/src/arrow/array/diff.cc @@ -639,42 +639,44 @@ class MakeFormatterImpl { auto fmt = fmt_str.c_str(); auto unit = checked_cast(*array.type()).unit(); auto value = checked_cast&>(array).Value(index); - using arrow_vendored::date::format; + // Using unqualified `format` directly would produce ambiguous + // lookup because of `std::format` (ARROW-15520). + namespace avd = arrow_vendored::date; using std::chrono::nanoseconds; using std::chrono::microseconds; using std::chrono::milliseconds; using std::chrono::seconds; if (AddEpoch) { - static arrow_vendored::date::sys_days epoch{arrow_vendored::date::jan / 1 / 1970}; + static avd::sys_days epoch{avd::jan / 1 / 1970}; switch (unit) { case TimeUnit::NANO: - *os << format(fmt, static_cast(value) + epoch); + *os << avd::format(fmt, static_cast(value) + epoch); break; case TimeUnit::MICRO: - *os << format(fmt, static_cast(value) + epoch); + *os << avd::format(fmt, static_cast(value) + epoch); break; case TimeUnit::MILLI: - *os << format(fmt, static_cast(value) + epoch); + *os << avd::format(fmt, static_cast(value) + epoch); break; case TimeUnit::SECOND: - *os << format(fmt, static_cast(value) + epoch); + *os << avd::format(fmt, static_cast(value) + epoch); break; } return; } switch (unit) { case TimeUnit::NANO: - *os << format(fmt, static_cast(value)); + *os << avd::format(fmt, static_cast(value)); break; case TimeUnit::MICRO: - *os << format(fmt, static_cast(value)); + *os << avd::format(fmt, static_cast(value)); break; case TimeUnit::MILLI: - *os << format(fmt, static_cast(value)); + *os << avd::format(fmt, static_cast(value)); break; case TimeUnit::SECOND: - *os << format(fmt, static_cast(value)); + *os << avd::format(fmt, static_cast(value)); break; } };