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

VectorIterator has broken assignment operator #349

Closed
sherm1 opened this issue Mar 16, 2015 · 3 comments
Closed

VectorIterator has broken assignment operator #349

sherm1 opened this issue Mar 16, 2015 · 3 comments
Assignees
Labels
Milestone

Comments

@sherm1
Copy link
Member

sherm1 commented Mar 16, 2015

The VectorIterator class keeps a reference to the Vector which prevents copy assignment from working. Nevertheless it contains a copy assignment operator which ends up making a deep copy of the Vector, rather then setting the new reference to be the same as the old one.

This is often a self-copy which is weird and may be slow but possibly harmless which is probably why this wasn't noticed. Valgrind caught it. I'm not sure if this causes any noticeable bugs.

@sherm1 sherm1 added the bug label Mar 16, 2015
@sherm1 sherm1 self-assigned this Mar 16, 2015
@sherm1 sherm1 added this to the Simbody 4.0 milestone Mar 16, 2015
sherm1 added a commit that referenced this issue Mar 16, 2015
I also added some comments and replaced some copying with references.
@sherm1
Copy link
Member Author

sherm1 commented Mar 16, 2015

Fixed in this commit.

@sherm1 sherm1 closed this as completed Mar 16, 2015
@chrisdembia
Copy link
Member

FYI IIRC github will close this issue even if you have Fixes #issue in your commit message.

@sherm1
Copy link
Member Author

sherm1 commented Mar 16, 2015

Thanks -- actually I meant to push this to a separate branch and make a PR but my click finger got away from me and I ended up pushing it right into the master branch by accident. Didn't seem worth undoing it at that point.

sherm1 added a commit that referenced this issue Jun 14, 2015
I also added some comments and replaced some copying with references.

(cherry picked from commit 4abb5c8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants