1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
|
From fb5a4f6c4158178608be475a07ddeaab59fff149 Mon Sep 17 00:00:00 2001
From: "Stephan T. Lavavej" <stl@microsoft.com>
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 `<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.)
Closes #12317 from StephanTLavavej/cxx20-format
Lead-authored-by: Stephan T. Lavavej <stl@microsoft.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
Signed-off-by: Antoine Pitrou <antoine@python.org>
---
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<const T&>(*array.type()).unit();
auto value = checked_cast<const NumericArray<T>&>(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<nanoseconds>(value) + epoch);
+ *os << avd::format(fmt, static_cast<nanoseconds>(value) + epoch);
break;
case TimeUnit::MICRO:
- *os << format(fmt, static_cast<microseconds>(value) + epoch);
+ *os << avd::format(fmt, static_cast<microseconds>(value) + epoch);
break;
case TimeUnit::MILLI:
- *os << format(fmt, static_cast<milliseconds>(value) + epoch);
+ *os << avd::format(fmt, static_cast<milliseconds>(value) + epoch);
break;
case TimeUnit::SECOND:
- *os << format(fmt, static_cast<seconds>(value) + epoch);
+ *os << avd::format(fmt, static_cast<seconds>(value) + epoch);
break;
}
return;
}
switch (unit) {
case TimeUnit::NANO:
- *os << format(fmt, static_cast<nanoseconds>(value));
+ *os << avd::format(fmt, static_cast<nanoseconds>(value));
break;
case TimeUnit::MICRO:
- *os << format(fmt, static_cast<microseconds>(value));
+ *os << avd::format(fmt, static_cast<microseconds>(value));
break;
case TimeUnit::MILLI:
- *os << format(fmt, static_cast<milliseconds>(value));
+ *os << avd::format(fmt, static_cast<milliseconds>(value));
break;
case TimeUnit::SECOND:
- *os << format(fmt, static_cast<seconds>(value));
+ *os << avd::format(fmt, static_cast<seconds>(value));
break;
}
};
|
