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

<bits/move.h> is a part of <utility>. #522

Closed
wants to merge 1 commit into from

Conversation

bungeman
Copy link
Contributor

@bungeman bungeman commented Mar 8, 2018

<bits/move.h> provides std::forward, std::move, and std::swap. These are defined to come from <utility> and not <algorithm>.

In c++11 and later std::swap comes from <utility>, previously it came from <algorithm>. Fortunately, <bits/move.h> is a c++11 only header, so if any symbols are being seen in it, that's where they should come from.

This was reported in #486.

@kimgr
Copy link
Contributor

kimgr commented Mar 8, 2018

Thanks! Any chance for a testcase?

Also, we have freestanding mappings in gcc.*.imp, could you make sure they're updated similarly?

@bungeman
Copy link
Contributor Author

bungeman commented Mar 8, 2018

Updated the *.imp, add the test from #486. I'm doing this between other things, so I just did all this in the github webui, resulting in all these one file commits. I think you can just "Squash and merge" and clean up the commit message. If not, let me know and I'll see if there's a good way to squash all this.

@kimgr
Copy link
Contributor

kimgr commented Mar 9, 2018

Neat, thanks! I'll take another look, test on my freebsd system and clean it up if I find anything.

<bits/move.h> provides std::forward, std::move, and std::swap. These are
defined to come from <utility> and not <algorithm>.

In c++11 and later std::swap comes from <utility>, previously it came from
<algorithm>. Fortunately, <bits/move.h> is a c++11 only header, so if any
symbols are being seen in it, that's where they should come from.

This was reported in include-what-you-use#486.
@bungeman
Copy link
Contributor Author

bungeman commented Mar 9, 2018

I made it back to a real machine so I squashed this merge request myself. Makes it easier to look at.

@kimgr
Copy link
Contributor

kimgr commented Mar 9, 2018

Thank you! I just gave this a whirl on FreeBSD. Of course the test fails :-/. The system C++ library on BSD is libc++ and all the builtin mappings are for libstdc++, so we don't have mappings and <type_traits> is suggested for std::move.

I'm not sure how I feel about this. On the one hand test coverage is nice, but on the other hand non-portable tests are a pain. We don't have good test infrastructure to detect these things either.

We actively try to keep new test cases independent of the environment by creating stub headers for constructs in the standard library, when we run into issues. But the mappings are naturally environment-specific.

Sigh. I'll rework your commit and remove the test (sorry about the wasted effort).

@kimgr
Copy link
Contributor

kimgr commented Mar 9, 2018

Did that and merged in 314fc2d. Thanks for the patch!

@kimgr kimgr closed this Mar 9, 2018
@bungeman bungeman deleted the patch-2 branch March 9, 2018 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants