-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
GH-38889: [C++] Fix redundant move warnings #41107
Conversation
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format?
or
In the case of PARQUET issues on JIRA the title also supports:
See also: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which compiler are you just using? Would this hit https://wg21.cmeerw.net/cwg/issue1579 problem?
The compiler is gcc 11.4 Thanks for the link. So reading through that I guess the issue is that we are returning |
|
|
@github-actions crossbow submit test-r-rstudio-r-base-4.1-opensuse153 |
Revision: 93364b9 Submitted crossbow builds: ursacomputing/crossbow @ actions-e7467d08dc
|
@github-actions crossbow submit -g cpp |
This comment was marked as outdated.
This comment was marked as outdated.
93364b9
to
59665b0
Compare
@github-actions crossbow submit -g cpp |
Revision: 59665b0 Submitted crossbow builds: ursacomputing/crossbow @ actions-a325efdea9 |
After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit 61625ad. There were 9 benchmark results indicating a performance regression:
The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them. |
Though the original issue was closed, I am also seeing this warning when compiling nanoarrow ``` [13/91] Compiling C++ object src/nanoarrow/utils_test.p/utils_test.cc.o FAILED: src/nanoarrow/utils_test.p/utils_test.cc.o c++ -Isrc/nanoarrow/utils_test.p -Isrc/nanoarrow -I../src/nanoarrow -Isrc -I../src -I/home/arrow-nanoarrow/arrow/include -fdiagnostics-color=always -D_GLIBCXX_ASSERTIONS=1 -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -std=c++17 -O3 -pthread -isystem../subprojects/googletest-1.14.0/googletest -isystemsubprojects/googletest-1.14.0/googletest -isystem../subprojects/googletest-1.14.0/googletest/include -MD -MQ src/nanoarrow/utils_test.p/utils_test.cc.o -MF src/nanoarrow/utils_test.p/utils_test.cc.o.d -o src/nanoarrow/utils_test.p/utils_test.cc.o -c ../src/nanoarrow/utils_test.cc In file included from ../src/nanoarrow/utils_test.cc:21: /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h: In member function ‘arrow::Result<std::pair<arrow::Decimal128, arrow::Decimal128> > arrow::Decimal128::Divide(const arrow::Decimal128&) const’: /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:83:21: error: redundant move in return statement [-Werror=redundant-move] 83 | return std::move(result); | ~~~~~~~~~^~~~~~~~ /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:83:21: note: remove ‘std::move’ call /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h: In member function ‘arrow::Result<arrow::Decimal128> arrow::Decimal128::Rescale(int32_t, int32_t) const’: /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:121:21: error: redundant move in return statement [-Werror=redundant-move] 121 | return std::move(out); | ~~~~~~~~~^~~~~ /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:121:21: note: remove ‘std::move’ call /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h: In member function ‘arrow::Result<arrow::Decimal256> arrow::Decimal256::Rescale(int32_t, int32_t) const’: /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:221:21: error: redundant move in return statement [-Werror=redundant-move] 221 | return std::move(out); | ~~~~~~~~~^~~~~ /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:221:21: note: remove ‘std::move’ call /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h: In member function ‘arrow::Result<std::pair<arrow::Decimal256, arrow::Decimal256> > arrow::Decimal256::Divide(const arrow::Decimal256&) const’: /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:238:21: error: redundant move in return statement [-Werror=redundant-move] 238 | return std::move(result); | ~~~~~~~~~^~~~~~~~ /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:238:21: note: remove ‘std::move’ call ``` * GitHub Issue: apache#38889 ### Rationale for this change Helps clean up build warnings when compiling with -Wextra ### What changes are included in this PR? Removed std::move from some return statements ### Are these changes tested? N/A - just ensured program compiles cleanly ### Are there any user-facing changes? No Authored-by: Will Ayd <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
Though the original issue was closed, I am also seeing this warning when compiling nanoarrow ``` [13/91] Compiling C++ object src/nanoarrow/utils_test.p/utils_test.cc.o FAILED: src/nanoarrow/utils_test.p/utils_test.cc.o c++ -Isrc/nanoarrow/utils_test.p -Isrc/nanoarrow -I../src/nanoarrow -Isrc -I../src -I/home/arrow-nanoarrow/arrow/include -fdiagnostics-color=always -D_GLIBCXX_ASSERTIONS=1 -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -std=c++17 -O3 -pthread -isystem../subprojects/googletest-1.14.0/googletest -isystemsubprojects/googletest-1.14.0/googletest -isystem../subprojects/googletest-1.14.0/googletest/include -MD -MQ src/nanoarrow/utils_test.p/utils_test.cc.o -MF src/nanoarrow/utils_test.p/utils_test.cc.o.d -o src/nanoarrow/utils_test.p/utils_test.cc.o -c ../src/nanoarrow/utils_test.cc In file included from ../src/nanoarrow/utils_test.cc:21: /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h: In member function ‘arrow::Result<std::pair<arrow::Decimal128, arrow::Decimal128> > arrow::Decimal128::Divide(const arrow::Decimal128&) const’: /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:83:21: error: redundant move in return statement [-Werror=redundant-move] 83 | return std::move(result); | ~~~~~~~~~^~~~~~~~ /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:83:21: note: remove ‘std::move’ call /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h: In member function ‘arrow::Result<arrow::Decimal128> arrow::Decimal128::Rescale(int32_t, int32_t) const’: /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:121:21: error: redundant move in return statement [-Werror=redundant-move] 121 | return std::move(out); | ~~~~~~~~~^~~~~ /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:121:21: note: remove ‘std::move’ call /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h: In member function ‘arrow::Result<arrow::Decimal256> arrow::Decimal256::Rescale(int32_t, int32_t) const’: /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:221:21: error: redundant move in return statement [-Werror=redundant-move] 221 | return std::move(out); | ~~~~~~~~~^~~~~ /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:221:21: note: remove ‘std::move’ call /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h: In member function ‘arrow::Result<std::pair<arrow::Decimal256, arrow::Decimal256> > arrow::Decimal256::Divide(const arrow::Decimal256&) const’: /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:238:21: error: redundant move in return statement [-Werror=redundant-move] 238 | return std::move(result); | ~~~~~~~~~^~~~~~~~ /home/arrow-nanoarrow/arrow/include/arrow/util/decimal.h:238:21: note: remove ‘std::move’ call ``` * GitHub Issue: apache#38889 ### Rationale for this change Helps clean up build warnings when compiling with -Wextra ### What changes are included in this PR? Removed std::move from some return statements ### Are these changes tested? N/A - just ensured program compiles cleanly ### Are there any user-facing changes? No Authored-by: Will Ayd <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
Though the original issue was closed, I am also seeing this warning when compiling nanoarrow
Rationale for this change
Helps clean up build warnings when compiling with -Wextra
What changes are included in this PR?
Removed std::move from some return statements
Are these changes tested?
N/A - just ensured program compiles cleanly
Are there any user-facing changes?
No