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.

Sign up for GitHub

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

Jump to bottom

[C++] util/small_vector_test.cc reports a self-move warning with MinGW #35651

Closed
kou opened this issue May 17, 2023 · 1 comment · Fixed by #35653 or #36328
Closed

[C++] util/small_vector_test.cc reports a self-move warning with MinGW #35651

kou opened this issue May 17, 2023 · 1 comment · Fixed by #35653 or #36328
Assignees
@kou
Labels
Component: C++ Type: bug
Milestone
13.0.0

Comments

@kou
Copy link
Member

kou commented May 17, 2023

Describe the bug, including details regarding any error messages, version, and platform.

https://github.com/apache/arrow/actions/runs/5006155150/jobs/8971042580#step:7:1154

D:/a/arrow/arrow/cpp/src/arrow/util/small_vector_test.cc:417:22: warning: moving 'moved_moved_ints' of type 'arrow::internal::TestSmallStaticVector<arrow::internal::VectorIntLikeParam<arrow::internal::StaticVectorTraits, arrow::MoveOnlyDataType> >::IntVectorType<5>' {aka 'arrow::internal::StaticVectorImpl<arrow::MoveOnlyDataType, 5, arrow::internal::StaticVectorStorage<arrow::MoveOnlyDataType, 5, true> >'} to itself [-Wself-move]
  417 |     moved_moved_ints = std::move(moved_moved_ints);
      |     ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
D:/a/arrow/arrow/cpp/src/arrow/util/small_vector_test.cc:417:22: note: remove 'std::move' call

Component(s)

C++
@kou kou added the Type: bug label May 17, 2023
kou added a commit to kou/arrow that referenced this issue May 17, 2023
@kou
apacheGH-35651: [C++] Don't use self-move with MinGW
dea9b23
@github-actions github-actions bot mentioned this issue May 17, 2023
Merged
@pitrou pitrou added this to the 13.0.0 milestone May 18, 2023
pitrou pushed a commit that referenced this issue May 18, 2023
@kou
GH-35651: [C++] Don't use self-move with MinGW (#35653)
3e4eaa9 
### Rationale for this change

Because MinGW g++ reports a warning:

```text
D:/a/arrow/arrow/cpp/src/arrow/util/small_vector_test.cc:417:22:
warning: moving 'moved_moved_ints' of type
'arrow::internal::TestSmallStaticVector<
  arrow::internal::VectorIntLikeParam<
    arrow::internal::StaticVectorTraits, arrow::MoveOnlyDataType> >::IntVectorType<5>'
{aka 'arrow::internal::StaticVectorImpl<
        arrow::MoveOnlyDataType, 5, arrow::internal::StaticVectorStorage<arrow::MoveOnlyDataType, 5, true> >'}
to itself [-Wself-move]
  417 |     moved_moved_ints = std::move(moved_moved_ints);
      |     ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
D:/a/arrow/arrow/cpp/src/arrow/util/small_vector_test.cc:417:22: note: remove 'std::move' call
```

### What changes are included in this PR?

Disable self-move code only for MinGW.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* Closes: #35651

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@zhjwpku
Copy link
Contributor

zhjwpku commented Jun 27, 2023

I got the same error on macos with gcc 13.1

/Users/zhjwpku/zhjwpku/arrow/cpp/src/arrow/util/small_vector_test.cc:418:22: error: moving 'moved_moved_ints' of type 'arrow::internal::TestSmallStaticVector<arrow::internal::VectorIntLikeParam<arrow::internal::SmallVectorTraits, arrow::MoveOnlyDataType> >::IntVectorType<5>' {aka 'arrow::internal::StaticVectorImpl<arrow::MoveOnlyDataType, 5, arrow::internal::SmallVectorStorage<arrow::MoveOnlyDataType, 5> >'} to itself [-Werror=self-move]
/Users/zhjwpku/zhjwpku/arrow/cpp/src/arrow/util/small_vector_test.cc:418:22: note: remove 'std::move' call

I checked the GCC 13 Release note, this is a newly added warning, I guess the warning with MinGW might be using
gcc 13 too.

zhjwpku added a commit to zhjwpku/arrow that referenced this issue Jun 27, 2023
@zhjwpku
apacheGH-35651: [C++] suppress self-move warning introduced in gcc 13
c06c18c 
PR apache#35653 had a fix for MinGW, but I think the real cause is that
gcc add a new warning -Wself-move.

see https://gcc.gnu.org/gcc-13/changes.html.

We should suppress it if we need to move to itself.

Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
@github-actions github-actions bot mentioned this issue Jun 27, 2023
Merged
pitrou pushed a commit that referenced this issue Jun 27, 2023
@zhjwpku
GH-35651: [C++] Suppress self-move warning introduced in gcc 13 (#36328)
8b4a548 
### Rationale for this change

PR #35653 had a fix for MinGW, but I think the real cause is that gcc add a new warning -Wself-move.

see https://gcc.gnu.org/gcc-13/changes.html.

We should suppress it if we need to move to itself.

### What changes are included in this PR?

suppress self-move warning by adding a cpp flag

### Are these changes tested?

Yes

### Are there any user-facing changes?

No

* Closes: #35651

Authored-by: Zhao Junwang <zhjwpku@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kou kou

Labels
Component: C++ Type: bug
Projects
None yet
Milestone
13.0.0
3 participants
@kou @pitrou @zhjwpku