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

[REVIEW] fix gcc-9 compilation error #6298

Merged
merged 3 commits into from
Sep 23, 2020

Conversation

rongou
Copy link
Contributor

@rongou rongou commented Sep 22, 2020

The std::move() call prevents copy elision:

/home/rou/src/cudf/cpp/src/dictionary/remove_keys.cu: In instantiation of ‘std::unique_ptr<cudf::column> cudf::dictionary::detail::_GLOBAL__N__46_tmpxft_00004b4a_00000000_7_remove_keys_cpp1_ii_8f7b9da6::remove_keys_fn(const cudf::dictionary_column_view&, KeysKeeper, rmm::mr::device_memory_resource*, cudaStream_t) [with KeysKeeper = __nv_dl_wrapper_t<__nv_dl_tag<std::unique_ptr<cudf::column> (*)(const cudf::dictionary_column_view&, const cudf::column_view&, rmm::mr::device_memory_resource*, CUstream_st*), cudf::dictionary::detail::remove_keys, 1>, const bool*>; cudaStream_t = CUstream_st*]’:
/home/rou/src/cudf/cpp/src/dictionary/remove_keys.cu:160:65:   required from here
/home/rou/src/cudf/cpp/src/dictionary/remove_keys.cu:83:27: error: moving a local object in a return statement prevents copy elision [-Werror=pessimizing-move]
   83 |       return std::move(positions);
      |                           ^
/home/rou/src/cudf/cpp/src/dictionary/remove_keys.cu:83:27: note: remove ‘std::move’ call
/home/rou/src/cudf/cpp/src/dictionary/remove_keys.cu: In instantiation of ‘std::unique_ptr<cudf::column> cudf::dictionary::detail::_GLOBAL__N__46_tmpxft_00004b4a_00000000_7_remove_keys_cpp1_ii_8f7b9da6::remove_keys_fn(const cudf::dictionary_column_view&, KeysKeeper, rmm::mr::device_memory_resource*, cudaStream_t) [with KeysKeeper = __nv_dl_wrapper_t<__nv_dl_tag<std::unique_ptr<cudf::column> (*)(const cudf::dictionary_column_view&, rmm::mr::device_memory_resource*, CUstream_st*), cudf::dictionary::detail::remove_unused_keys, 1>, const bool*>; cudaStream_t = CUstream_st*]’:
/home/rou/src/cudf/cpp/src/dictionary/remove_keys.cu:186:65:   required from here
/home/rou/src/cudf/cpp/src/dictionary/remove_keys.cu:83:27: error: moving a local object in a return statement prevents copy elision [-Werror=pessimizing-move]
/home/rou/src/cudf/cpp/src/dictionary/remove_keys.cu:83:27: note: remove ‘std::move’ call
cc1plus: all warnings being treated as errors
CMakeFiles/cudf.dir/build.make:2668: recipe for target 'CMakeFiles/cudf.dir/src/dictionary/remove_keys.cu.o' failed
make[2]: *** [CMakeFiles/cudf.dir/src/dictionary/remove_keys.cu.o] Error 1
CMakeFiles/Makefile2:230: recipe for target 'CMakeFiles/cudf.dir/all' failed
make[1]: *** [CMakeFiles/cudf.dir/all] Error 2
Makefile:148: recipe for target 'all' failed
make: *** [all] Error 2

@davidwendt @jrhemstad @kkraus14

@rongou rongou requested a review from a team as a code owner September 22, 2020 21:10
@GPUtester
Copy link
Collaborator

Can one of the admins verify this patch?

@GPUtester
Copy link
Collaborator

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

@devavret
Copy link
Contributor

Github shows only 1 line changed. And that's the changelog. Where's the code?

@rongou
Copy link
Contributor Author

rongou commented Sep 22, 2020

Ah I was trying to reproduce the error.

@kkraus14 kkraus14 added 3 - Ready for Review Ready for review by team libcudf Affects libcudf (C++/CUDA) code. labels Sep 22, 2020
@codecov
Copy link

codecov bot commented Sep 23, 2020

Codecov Report

Merging #6298 into branch-0.16 will increase coverage by 0.28%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.16    #6298      +/-   ##
===============================================
+ Coverage        84.45%   84.74%   +0.28%     
===============================================
  Files               82       82              
  Lines            13846    14176     +330     
===============================================
+ Hits             11694    12013     +319     
- Misses            2152     2163      +11     
Impacted Files Coverage Δ
python/dask_cudf/dask_cudf/io/parquet.py 91.35% <0.00%> (-0.85%) ⬇️
python/cudf/cudf/_version.py 44.80% <0.00%> (-0.72%) ⬇️
python/cudf/cudf/io/parquet.py 91.73% <0.00%> (-0.50%) ⬇️
python/cudf/cudf/core/series.py 90.96% <0.00%> (-0.37%) ⬇️
python/cudf/cudf/utils/applyutils.py 98.74% <0.00%> (-0.01%) ⬇️
python/cudf/cudf/utils/ioutils.py 85.84% <0.00%> (ø)
python/cudf/cudf/utils/docutils.py 100.00% <0.00%> (ø)
python/dask_cudf/dask_cudf/io/csv.py 95.52% <0.00%> (ø)
python/cudf/cudf/core/column/string.py 86.05% <0.00%> (ø)
python/dask_cudf/dask_cudf/io/tests/test_csv.py 100.00% <0.00%> (ø)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a16c1f...c27594d. Read the comment docs.

@harrism
Copy link
Member

harrism commented Sep 23, 2020

rerun tests

1 similar comment
@galipremsagar
Copy link
Contributor

rerun tests

@galipremsagar galipremsagar added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Sep 23, 2020
@kkraus14 kkraus14 merged commit 5976d74 into rapidsai:branch-0.16 Sep 23, 2020
@rongou rongou deleted the fix-gcc9-error branch October 9, 2020 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants