Skip to content

Conversation

@ccotter
Copy link
Contributor

@ccotter ccotter commented Jul 3, 2023

Each of these looks like they were meant tobe a move.

Each of these looks like they were meant tobe a move.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 3, 2023
@ccotter
Copy link
Contributor Author

ccotter commented Jul 3, 2023

I've got an unmerged clang-tidy check in my local build that finds places where forward is used on a non-forwarding reference. Ref: #548

Copy link
Contributor

@ispeters ispeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice clean-up.

public:
explicit type(Operation& op, Receiver&& r) :
op_(op), receiver_{std::forward<Receiver>(r)}
op_(op), receiver_{std::move(r)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is better than it was, but this whole file looks like it needs to have the value categories of the various forwarded arguments reconsidered.

@ispeters ispeters merged commit d907a30 into facebookexperimental:main Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants