[ 
https://issues.apache.org/jira/browse/ARROW-15520?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Antoine Pitrou updated ARROW-15520:
-----------------------------------
    Summary: [C++] Unqualified format() calls are ambiguous in C++20  (was: 
Unqualified format() calls are ambiguous in C++20)

> [C++] Unqualified format() calls are ambiguous in C++20
> -------------------------------------------------------
>
>                 Key: ARROW-15520
>                 URL: https://issues.apache.org/jira/browse/ARROW-15520
>             Project: Apache Arrow
>          Issue Type: Bug
>          Components: C++
>    Affects Versions: 7.0.0
>         Environment: Visual Studio 2022 17.2 Preview 2
>            Reporter: Stephan T. Lavavej
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> I work on MSVC's C++ Standard Library implementation, and we regularly build 
> open-source projects, including Apache Arrow, with development versions of 
> the MSVC toolset in order to find and fix compiler/library bugs before they 
> can cause problems for our programmer-users like you. This also allows us to 
> provide advance notice of breaking changes in the C++ Standard that will 
> affect you, which is the case here.
> We recently implemented C++20's std::format(), followed by the C++20 Defect 
> Report [P2418R2|https://wg21.link/P2418R2] "Add Support For 
> std::generator-like Types To std::format" with 
> [microsoft/STL#2323|https://github.com/microsoft/STL/pull/2323]. This 
> prevents Apache Arrow from compiling with the latest C++ Standard mode 
> enabled, because MakeTimeFormatter() in diff.cc contains unqualified calls: 
> [https://github.com/apache/arrow/blob/56e270fda7f5647a157acd1e428d9735d6399881/cpp/src/arrow/array/diff.cc#L636-L681]
> (The issue involves Argument-Dependent Lookup. The using-declaration `using 
> arrow_vendored::date::format;` means that the following unqualified calls to 
> `format()` will consider the overload in the `arrow_vendored::date` 
> namespace, which is the desired overload. However, because the arguments are 
> `std::chrono::duration` types, `std` is considered an "associated namespace", 
> so it will also be searched for overloads. Our implementation of the chrono 
> header includes the new format header (as permitted by the Standard - we do 
> this because chrono types are formattable in C++20), so the `std::format` 
> overload is visible. Finally, the signature change required by the P2418R2 
> paper makes `std::format` a highly greedy "perfect forwarding" signature, so 
> it's ambiguous with the desired `arrow_vendored::date::format` overload.)
> The full steps to reproduce are:
>  # Build the microsoft/STL repo, and update the INCLUDE/LIB/etc. environment 
> variables so that it can be consumed. (Or wait for Visual Studio 2022 17.2 
> Preview 2 to ship, as it will contain these changes.)
>  # Configure Apache Arrow with the latest C++ Standard version: cmake -G 
> Ninja -S . -B build -DCMAKE_CXX_STANDARD=23 (note that C++23 must be selected 
> at this time even though std::format() is C++20 - it's a long story)
>  # diff.cc fails to compile with:
>  
> {noformat}
> ninja: Entering directory `build'
> [13/191] Building CXX object 
> src\arrow\CMakeFiles\arrow_static.dir\array\diff.cc.obj
> FAILED: src/arrow/CMakeFiles/arrow_static.dir/array/diff.cc.obj
> C:\PROGRA~1\MIB055~1\2022\Preview\VC\Tools\MSVC\1431~1.311\bin\Hostx64\x64\cl.exe
>   /nologo /TP -DARROW_EXPORTING -DARROW_HAVE_RUNTIME_AVX2 
> -DARROW_HAVE_RUNTIME_AVX512 -DARROW_HAVE_RUNTIME_BMI2 
> -DARROW_HAVE_RUNTIME_SSE4_2 -DARROW_HAVE_SSE4_2 -DARROW_STATIC 
> -DARROW_WITH_TIMING_TESTS -DURI_STATIC_BUILD -D_CRT_SECURE_NO_WARNINGS 
> -D_ENABLE_EXTENDED_ALIGNED_STORAGE -IC:\Temp\arrow\cpp\build\src 
> -IC:\Temp\arrow\cpp\src -IC:\Temp\arrow\cpp\src\generated 
> -IC:\Temp\arrow\cpp\thirdparty\flatbuffers\include 
> -IC:\Temp\arrow\cpp\build\boost_ep-prefix\src\boost_ep 
> -IC:\Temp\arrow\cpp\build\xsimd_ep\src\xsimd_ep-install\include 
> -IC:\Temp\arrow\cpp\thirdparty\hadoop\include /DWIN32 /D_WINDOWS  /GR /EHsc 
> /D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING   /EHsc /wd5105 /bigobj /utf-8 
> /W3 /wd4800 /wd4996 /wd4065   /MD /O2 /Ob2 /DNDEBUG -std:c++latest 
> /showIncludes /Fosrc\arrow\CMakeFiles\arrow_static.dir\array\diff.cc.obj 
> /Fdsrc\arrow\CMakeFiles\arrow_static.dir\arrow_static.pdb /FS -c 
> C:\Temp\arrow\cpp\src\arrow\array\diff.cc
> C:\Temp\arrow\cpp\src\arrow\array\diff.cc(652): error C2666: 
> 'arrow_vendored::date::format': 2 overloads have similar conversions
> C:\Temp\arrow\cpp\src\arrow/vendored/datetime/date.h(6264): note: could be 
> 'std::basic_string<char,std::char_traits<char>,std::allocator<char>> 
> arrow_vendored::date::format<_Elem,std::chrono::time_point<std::chrono::system_clock,std::chrono::duration<__int64,std::nano>>>(const
>  CharT *,const Streamable &)'
>         with
>         [
>             _Elem=char,
>             CharT=char,
>             
> Streamable=std::chrono::time_point<std::chrono::system_clock,std::chrono::duration<__int64,std::nano>>
>         ]
> C:\GitHub\STL\out\build\x64\out\inc\format(3100): note: or       
> 'std::wstring 
> std::format<std::chrono::time_point<std::chrono::system_clock,std::chrono::duration<__int64,std::nano>>>(const
>  
> std::_Basic_format_string<wchar_t,std::chrono::time_point<std::chrono::system_clock,std::chrono::duration<__int64,std::nano>>>,std::chrono::time_point<std::chrono::system_clock,std::chrono::duration<__int64,std::nano>>
>  &&)' [found using argument-dependent lookup]
> C:\GitHub\STL\out\build\x64\out\inc\format(3095): note: or       'std::string 
> std::format<std::chrono::time_point<std::chrono::system_clock,std::chrono::duration<__int64,std::nano>>>(const
>  
> std::_Basic_format_string<char,std::chrono::time_point<std::chrono::system_clock,std::chrono::duration<__int64,std::nano>>>,std::chrono::time_point<std::chrono::system_clock,std::chrono::duration<__int64,std::nano>>
>  &&)' [found using argument-dependent lookup]
> C:\Temp\arrow\cpp\src\arrow\array\diff.cc(680): note: while trying to match 
> the argument list '(const _Elem *, 
> std::chrono::time_point<std::chrono::system_clock,std::chrono::duration<__int64,std::nano>>)'
>         with
>         [
>             _Elem=char
>         ]
> C:\Temp\arrow\cpp\src\arrow\array\diff.cc(680): note: note: qualification 
> adjustment (const/volatile) may be causing the ambiguity
> C:\Temp\arrow\cpp\src\arrow\array\diff.cc(451): note: see reference to 
> function template instantiation 'arrow::Formatter 
> arrow::MakeFormatterImpl::MakeTimeFormatter<arrow::TimestampType,true>(const 
> std::string &)' being compiled
> [...]{noformat}
> The fix is very simple: remove the using-declaration and explicitly qualify 
> each call. I will submit a pull request on GitHub for this.
>  
> (Even if Apache Arrow isn't planning on using C++20 any time soon, making 
> this change will make it easier for MSVC to continue validating Apache Arrow 
> with the latest toolset and Standard changes, and it will remove a potential 
> future headache if and when Apache Arrow does migrate to later Standard 
> versions.)



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to