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

Thread.h: move copy constructor and assignment op #5133

Merged
merged 1 commit into from Nov 2, 2015

Conversation

stiopaa1
Copy link
Contributor

@stiopaa1 stiopaa1 commented Jul 2, 2015

Move copy constructor and assignemnt operator under the private access
specifier. They are not defined and it makes it confusing when they are
under public. They also don't need const and explicit specifiers.

Signed-off-by: Michal Jarzabek stiopa@gmail.com

@ghost ghost self-assigned this Jul 3, 2015
@ghost
Copy link

ghost commented Aug 29, 2015

The way I understand it is that explicit is a safeguard to prevent code that would rely on implicit constructors by mistake.

@stiopaa1
Copy link
Contributor Author

What difference does it make if the copy constructor is not defined?
Even when copy constructor is called with a type that is not implicitly converted to Thread we will get an error during the linking stage?
Or maybe I am missing something?

@tchaikov
Copy link
Contributor

@stiopaa1 i tend to agree with you. but since we are using C++11 in master, probably we could "delete" the default copy ctor and assignment operator, see http://www.stroustrup.com/C++11FAQ.html#default .

@stiopaa1
Copy link
Contributor Author

Agreed, will change that to use c++11's 'delete'.
When this was submitted it was still c++03.

@ghost ghost assigned tchaikov and unassigned ghost Sep 2, 2015
Disable copy constructor and assignment operator.
They are currently not defined, so any attempt to use them will
result in linker error.

Signed-off-by: Michal Jarzabek <stiopa@gmail.com>
tchaikov added a commit that referenced this pull request Nov 2, 2015
Thread.h: disable copy constructor and assignment op

Reviewed-by: Kefu Chai <kchai@redhat.com>
@tchaikov tchaikov merged commit ee029eb into ceph:master Nov 2, 2015
@tchaikov
Copy link
Contributor

tchaikov commented Nov 2, 2015

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants