Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
yew1eb opened this issue Apr 17, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@yew1eb
Copy link

yew1eb commented Apr 17, 2024

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 yew1eb added the bug Something isn't working label Apr 17, 2024
@yew1eb yew1eb changed the title Fix -Wpessimizing-move in TypeWithId.cpp: build error: moving a temporary object prevents copy elision [-Wpessimizing-move] Apr 17, 2024
@yew1eb
Copy link
Author

yew1eb commented Apr 17, 2024

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

@zhouyifan279
Copy link

zhouyifan279 commented Apr 24, 2024

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
Copy link
Author

yew1eb commented Apr 24, 2024

cc @zhouyuan, thanks.

@zhouyuan
Copy link
Collaborator

@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
Copy link

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

@zhouyuan
Copy link
Collaborator

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

thanks
-yuan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants