GH-38889: [C++] Fix redundant move warnings

[WillAyd]

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: [C++] GCC complains about redundant move #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

[github-actions]

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?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

• Other pull requests
• Contribution Guidelines - Contributing Overview

[mapleFU]

Which compiler are you just using? Would this hit https://wg21.cmeerw.net/cwg/issue1579 problem?

[WillAyd]

The compiler is gcc 11.4

Thanks for the link. So reading through that I guess the issue is that we are returning std::pair<Decimal128, Decimal128> from a function declared to return type Result<std::pair<Decimal128, Decimal128>> and that may not elide the copy?

[github-actions]

⚠️ GitHub issue #38889 has been automatically assigned in GitHub to PR creator.

[github-actions]

⚠️ GitHub issue #38889 has been automatically assigned in GitHub to PR creator.

[mapleFU]

@github-actions crossbow submit test-r-rstudio-r-base-4.1-opensuse153

[github-actions]

Revision: 93364b9

Submitted crossbow builds: ursacomputing/crossbow @ actions-e7467d08dc

Task	Status
test-r-rstudio-r-base-4.1-opensuse153

[pitrou]

@github-actions crossbow submit -g cpp

[pitrou]

@github-actions crossbow submit -g cpp

[github-actions]

Revision: 59665b0

Submitted crossbow builds: ursacomputing/crossbow @ actions-a325efdea9

Task	Status
test-alpine-linux-cpp
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-cuda-cpp
test-debian-12-cpp-amd64
test-debian-12-cpp-i386
test-fedora-39-cpp
test-ubuntu-20.04-cpp
test-ubuntu-20.04-cpp-bundled
test-ubuntu-20.04-cpp-minimal-with-formats
test-ubuntu-20.04-cpp-thread-sanitizer
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-emscripten
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-gcc-14

[conbench-apache-arrow]

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:

• Commit Run on ursa-i9-9960x at 2024-04-17 03:22:18Z
  • tpch (R) with engine=arrow, format=native, language=R, memory_map=False, query_id=TPCH-16, scale_factor=1
  • file-read (R) with compression=lz4, dataset=nyctaxi_2010-01, file_type=feather, language=R, output_type=table
• and 7 more (see the report linked below)

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.