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

Joint has the default copy constructor #524

Closed
mkoval opened this issue Oct 15, 2015 · 8 comments
Closed

Joint has the default copy constructor #524

mkoval opened this issue Oct 15, 2015 · 8 comments
Milestone
DART 5.1.1

Comments

@mkoval
Copy link
Collaborator

mkoval commented Oct 15, 2015

I noticed that Joint does not = delete its copy constructor. I believe that all of the fields in Joint are copyable, so this means that the default copy (and, possibly, move) constructor is being implicitly defined by the compiler.

I suspect that is not intentional (and potentially dangerous!) because every Joint is supposed to be owned by it's child BodyNode.
@mxgrey
Copy link
Member

mxgrey commented Oct 15, 2015

It's definitely not intentional, and it's definitely dangerous. I'll submit a pull request to correct this later today.

@mxgrey
Copy link
Member

mxgrey commented Oct 15, 2015

It's also worth checking if this is an issue for BodyNode and Skeleton, since none of those should permit a copy construction.

@mxgrey mxgrey mentioned this issue Oct 15, 2015
Merged
@mkoval
Copy link
Collaborator Author

mkoval commented Oct 16, 2015

@mxgrey fixed this in #525. Thanks!

@mkoval mkoval closed this as completed Oct 16, 2015
@jslee02 jslee02 added this to the DART 5.1.1 milestone Oct 16, 2015
@mkoval
Copy link
Collaborator Author

mkoval commented Oct 23, 2015

MultiDofJoint and ZeroDofJoint still have copy constructors. 😄

@mkoval mkoval reopened this Oct 23, 2015
@mxgrey
Copy link
Member

mxgrey commented Oct 23, 2015

I'm pretty sure they'd technically be deleted by default since they inherit from a class whose copy constructor is deleted. Still, it would be good to be explicit about it.

@mkoval
Copy link
Collaborator Author

mkoval commented Oct 23, 2015

They're not - I discovered this because I'm using libclang to parse the DART source code and they're listed in the output. 😆

@mkoval
Copy link
Collaborator Author

mkoval commented Oct 23, 2015

DegreeOfFreedom also has a copy constructor.

@jslee02
Copy link
Member

jslee02 commented Oct 25, 2015

This seems to be resolved by #539.

@jslee02 jslee02 closed this as completed Oct 25, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
DART 5.1.1
Development

No branches or pull requests
3 participants
@mkoval @mxgrey @jslee02