build error: moving a temporary object prevents copy elision [-Wpessimizing-move]

yew1eb commented 7 months ago

Bug description

[393/1774] Building CXX object velox/dwio/common/CMakeFiles/velox_dwio_common.dir/TypeWithId.cpp.o
FAILED: velox/dwio/common/CMakeFiles/velox_dwio_common.dir/TypeWithId.cpp.o 
/opt/homebrew/bin/ccache /Library/Developer/CommandLineTools/usr/bin/c++ -DBOOST_ATOMIC_DYN_LINK -DBOOST_ATOMIC_NO_LIB -DBOOST_CONTEXT_DYN_LINK -DBOOST_CONTEXT_NO_LIB -DBOOST_FILESYSTEM_DYN_LINK -DBOOST_FILESYSTEM_NO_LIB -DBOOST_PROGRAM_OPTIONS_DYN_LINK -DBOOST_PROGRAM_OPTIONS_NO_LIB -DBOOST_REGEX_DYN_LINK -DBOOST_REGEX_NO_LIB -DBOOST_SYSTEM_DYN_LINK -DBOOST_SYSTEM_NO_LIB -DBOOST_THREAD_DYN_LINK -DBOOST_THREAD_NO_LIB -DFOLLY_HAVE_INT128_T=1 -DGFLAGS_IS_A_DLL=0 -I/Users/yew1eb/workspaces/oap-velox/. -I/Users/yew1eb/workspaces/oap-velox/velox/external/xxhash -isystem /usr/local/include -isystem /Users/yew1eb/workspaces/oap-velox/velox -isystem /Users/yew1eb/workspaces/oap-velox/velox/external -isystem /opt/homebrew/include -isystem /opt/homebrew/Cellar/openssl@3/3.2.1/include -mcpu=apple-m1+crc -std=c++17 -fvisibility=hidden -D USE_VELOX_COMMON_BASE -D HAS_UNCAUGHT_EXCEPTIONS -Wall -Wextra -Wno-unused        -Wno-unused-parameter        -Wno-sign-compare        -Wno-ignored-qualifiers        -Wno-range-loop-analysis          -Wno-mismatched-tags          -Wno-nullability-completeness -Werror -O3 -DNDEBUG -std=gnu++17 -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX14.4.sdk -mmacosx-version-min=14.3 -fPIC -fcolor-diagnostics -MD -MT velox/dwio/common/CMakeFiles/velox_dwio_common.dir/TypeWithId.cpp.o -MF velox/dwio/common/CMakeFiles/velox_dwio_common.dir/TypeWithId.cpp.o.d -o velox/dwio/common/CMakeFiles/velox_dwio_common.dir/TypeWithId.cpp.o -c /Users/yew1eb/workspaces/oap-velox/velox/dwio/common/TypeWithId.cpp
/Users/yew1eb/workspaces/oap-velox/velox/dwio/common/TypeWithId.cpp:89:27: error: moving a temporary object prevents copy elision [-Werror,-Wpessimizing-move]
    children.emplace_back(std::move(child->duplicate(nameAsLowerCase)));
                          ^
/Users/yew1eb/workspaces/oap-velox/velox/dwio/common/TypeWithId.cpp:89:27: note: remove std::move call here
    children.emplace_back(std::move(child->duplicate(nameAsLowerCase)));
                          ^~~~~~~~~~                                 ~
1 error generated.

System information

ProductName: macOS ProductVersion: 14.3.1 BuildVersion: 23D60

-- Building using CMake version: 3.29.2 -- The C compiler identification is AppleClang 15.0.0.15000309 -- The CXX compiler identification is AppleClang 15.0.0.15000309

Relevant logs

No response

yew1eb commented 7 months ago

I have fixed the TypeWithId.cpp file, and now the build passes successfully.

zhouyifan279 commented 7 months ago

Bug still exists in branch 2024_04_23

System information

ProductName: macOS ProductVersion: 14.4.1

-- Building using CMake version: 3.29.2 -- The C compiler identification is AppleClang 15.0.0.15000309 -- The CXX compiler identification is AppleClang 15.0.0.15000309

yew1eb commented 7 months ago

cc @zhouyuan, thanks.

zhouyuan commented 7 months ago

@yew1eb @zhouyifan279 thanks for reporting! Honestly gluten CI only do x86 related tests so it's not discovered. Based on the log, it seems like a bug/feature on the clang compiler, could you please create a patch on Meta/Velox so more Mac users can help to test?

thanks, -yuan

zhouyifan279 commented 7 months ago

@zhouyuan This error does not exist in https://github.com/facebookincubator/velox. It is introduced in this forked version by commit https://github.com/oap-project/velox/commit/089c1b16012ac48c29bc6771f304a3a5c7ec8723. I think the patch should be created in this project.

zhouyuan commented 7 months ago

@zhouyifan279 Thanks for the check, rui will kindly help to fix this in the upstream patch. thanks @rui-mo

thanks -yuan

oap-project / velox