[ARROW-12325] [C++] [CI] Nightly gandiva build failing due to failure of compiler to move return value

• Type: Improvement
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 4.0.0
• Component/s: C++, Continuous Integration
• Labels: pull-request-available
• External issue URL: https://github.com/apache/arrow/issues/28126
• Assignee: Weston Pace
• Reporter: Weston Pace
• Created: 10/Apr/21 03:59
• Updated: 11/Jan/23 08:25
• Resolved: 21/Apr/21 01:42

Description

The nightly gandiva build is failing with the below build error.

 

According to vague sources (https://channel9.msdn.com/Events/GoingNative/2013/Don-t-Help-the-Compiler#c635161962798701359) and some experimentation (https://gcc.godbolt.org/z/x138zevhE) it appears that GCC 4.8.2 will not apply a move to a return value if that value is being implicitly cast to another type (more specifically, it will not use the implicit rvalue constructor of the return type).

 

This is a pretty common occurrence in Arrow given so many of our returns are Result<...> but more specifically it is causing a test with a MoveOnlyDataType to fail.

 

FAILED: src/arrow/util/CMakeFiles/arrow-utility-test.dir/stl_util_test.cc.o 

  

  
    
    /usr/bin/ccache /opt/rh/devtoolset-2/root/usr/bin/c++ -DARROW_HAVE_RUNTIME_SSE4_2 -DARROW_HAVE_SSE4_2 -DARROW_JEMALLOC -DARROW_JEMALLOC_INCLUDE_DIR="" -DARROW_WITH_RE2 -DARROW_WITH_TIMING_TESTS -DURI_STATIC_BUILD -Isrc -I/arrow/cpp/src -I/arrow/cpp/src/generated -isystem /arrow/cpp/thirdparty/flatbuffers/include -isystem /arrow_boost_dist/include -isystem gflags_ep-prefix/src/gflags_ep/include -isystem jemalloc_ep-prefix/src -isystem rapidjson_ep/src/rapidjson_ep-install/include -isystem xsimd_ep/src/xsimd_ep-install/include -isystem re2_ep-install/include -isystem /arrow/cpp/thirdparty/hadoop/include -O3 -DNDEBUG  -Wall -Wno-attributes -msse4.2  -O3 -DNDEBUG -fPIE -pthread -std=c++11 -MD -MT src/arrow/util/CMakeFiles/arrow-utility-test.dir/stl_util_test.cc.o -MF src/arrow/util/CMakeFiles/arrow-utility-test.dir/stl_util_test.cc.o.d -o src/arrow/util/CMakeFiles/arrow-utility-test.dir/stl_util_test.cc.o -c /arrow/cpp/src/arrow/util/stl_util_test.cc

  

  
    
    In file included from /opt/rh/devtoolset-2/root/usr/include/c++/4.8.2/vector:62:0,

  

  
    
                     from /arrow/cpp/src/arrow/util/stl_util_test.cc:19:

  

  
    
    /opt/rh/devtoolset-2/root/usr/include/c++/4.8.2/bits/stl_construct.h: In instantiation of ‘void std::_Construct(_T1*, _Args&& ...) [with _T1 = arrow::MoveOnlyDataType; _Args = {const arrow::MoveOnlyDataType&}]’:

  

  
    
    /opt/rh/devtoolset-2/root/usr/include/c++/4.8.2/bits/stl_uninitialized.h:75:53:   required from ‘static _ForwardIterator std::__uninitialized_copy<_TrivialValueTypes>::__uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = __gnu_cxx::__normal_iterator<const arrow::MoveOnlyDataType*, std::vector<arrow::MoveOnlyDataType, std::allocator<arrow::MoveOnlyDataType> > >; _ForwardIterator = arrow::MoveOnlyDataType*; bool _TrivialValueTypes = false]’

  

  
    
    /opt/rh/devtoolset-2/root/usr/include/c++/4.8.2/bits/stl_uninitialized.h:117:41:   required from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = __gnu_cxx::__normal_iterator<const arrow::MoveOnlyDataType*, std::vector<arrow::MoveOnlyDataType, std::allocator<arrow::MoveOnlyDataType> > >; _ForwardIterator = arrow::MoveOnlyDataType*]’

  

  
    
    /opt/rh/devtoolset-2/root/usr/include/c++/4.8.2/bits/stl_uninitialized.h:258:63:   required from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, std::allocator<_Tp>&) [with _InputIterator = __gnu_cxx::__normal_iterator<const arrow::MoveOnlyDataType*, std::vector<arrow::MoveOnlyDataType, std::allocator<arrow::MoveOnlyDataType> > >; _ForwardIterator = arrow::MoveOnlyDataType*; _Tp = arrow::MoveOnlyDataType]’

  

  
    
    /opt/rh/devtoolset-2/root/usr/include/c++/4.8.2/bits/stl_vector.h:316:32:   required from ‘std::vector<_Tp, _Alloc>::vector(const std::vector<_Tp, _Alloc>&) [with _Tp = arrow::MoveOnlyDataType; _Alloc = std::allocator<arrow::MoveOnlyDataType>]’

  

  
    
    /arrow/cpp/src/arrow/result.h:446:5:   required from ‘void arrow::Result<T>::ConstructValue(U&&) [with U = std::vector<arrow::MoveOnlyDataType, std::allocator<arrow::MoveOnlyDataType> >&; T = std::vector<arrow::MoveOnlyDataType, std::allocator<arrow::MoveOnlyDataType> >]’

  

  
    
    /arrow/cpp/src/arrow/result.h:171:42:   required from ‘arrow::Result<T>::Result(U&&) [with U = std::vector<arrow::MoveOnlyDataType, std::allocator<arrow::MoveOnlyDataType> >&; E = void; T = std::vector<arrow::MoveOnlyDataType, std::allocator<arrow::MoveOnlyDataType> >]’

  

  
    
    /arrow/cpp/src/arrow/util/vector.h:133:10:   required from ‘arrow::Result<std::vector<_RealType> > arrow::internal::UnwrapOrRaise(std::vector<arrow::Result<T> >&&) [with T = arrow::MoveOnlyDataType]’

  

  
    
    /arrow/cpp/src/arrow/util/stl_util_test.cc:160:3:   required from here

  

  
    
  
    
    /opt/rh/devtoolset-2/root/usr/include/c++/4.8.2/bits/stl_construct.h:75:7: error: use of deleted function ‘arrow::MoveOnlyDataType::MoveOnlyDataType(const arrow::MoveOnlyDataType&)’

  

  
    
         { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }

  

  
    
           ^

  

  
    
    In file included from /arrow/cpp/src/arrow/util/stl_util_test.cc:24:0:

  

  
    
    /arrow/cpp/src/arrow/testing/gtest_util.h:527:3: error: declared here

  

  
    
       MoveOnlyDataType(const MoveOnlyDataType& other) = delete;

  

  
    
       ^

A simple fix is to `std::move` the return value but that opens up a whole can of worms.  I'm open to alternative solutions.

Issue Links

• links to: GitHub Pull Request #9972
• links to: GitHub Pull Request #9975