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

Remove redundant move #1474

Merged
merged 1 commit into from
Dec 29, 2018
Merged

Remove redundant move #1474

merged 1 commit into from
Dec 29, 2018

Conversation

kraj
Copy link
Contributor

@kraj kraj commented Dec 18, 2018

fixes errors like below

/mnt/a/yoe/build/tmp/work/aarch64-yoe-linux-musl/catch2/2.5.0-r0/git/include/internal/catch_session.cpp:52:29:
error: redundant move in return statement [-Werror=redundant-move]
52 | return std::move(multi);
| ~~~~~~~~~^~~~~~~
/mnt/a/yoe/build/tmp/work/aarch64-yoe-linux-musl/catch2/2.5.0-r0/git/include/internal/catch_session.cpp:52:29:
note: remove 'std::move' call

Signed-off-by: Khem Raj raj.khem@gmail.com

Description

GitHub Issues

@horenmar
Copy link
Member

As you can see on Travis, it is not quite redundant enough if you are using older gcc+libstdc++ combination.

@kraj
Copy link
Contributor Author

kraj commented Dec 21, 2018

@horenmar Yes, I see, may be we can add conditionals for checking Clang >= 3.9 and gcc >= 5 to enable it. Otherwise keep the std::move

@codecov
Copy link

codecov bot commented Dec 27, 2018

Codecov Report

Merging #1474 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1474      +/-   ##
==========================================
+ Coverage   80.38%   80.39%   +0.01%     
==========================================
  Files         121      121              
  Lines        3425     3426       +1     
==========================================
+ Hits         2753     2754       +1     
  Misses        672      672

@codecov
Copy link

codecov bot commented Dec 27, 2018

Codecov Report

Merging #1474 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1474      +/-   ##
==========================================
+ Coverage   80.38%   80.42%   +0.04%     
==========================================
  Files         121      121              
  Lines        3425     3422       -3     
==========================================
- Hits         2753     2752       -1     
+ Misses        672      670       -2

@horenmar
Copy link
Member

I took a slightly different approach, can you check if it eliminates the warning on your machine?

@kraj
Copy link
Contributor Author

kraj commented Dec 27, 2018

I took a slightly different approach, can you check if it eliminates the warning on your machine?

The alternate approach works for me.

Tested- by: Khem Raj raj.khem@gmail.com

Signed-off-by: Khem Raj <raj.khem@gmail.com>
@horenmar
Copy link
Member

Ok, great.

@horenmar horenmar merged commit 799c7a2 into catchorg:master Dec 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants