Skip to content

vec: avoid some unsafe code in MoveIterator's dtor #10998

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

Merged
merged 1 commit into from
Dec 17, 2013
Merged

vec: avoid some unsafe code in MoveIterator's dtor #10998

merged 1 commit into from
Dec 17, 2013

Conversation

thestinger
Copy link
Contributor

No description provided.

@erickt
Copy link
Contributor

erickt commented Dec 16, 2013

This for loop is what was causing my 30% hit I mentioned in #10988. Would it be possible to wrap the for loop in a needs_drop::<T>() if-block (assuming that's the right intrinsic)? That appears to allow my micro-benchmark to generate the same code as push_all_move.

edit: I'm guessing this is needed so the values are appropriately dropped, so it is needed :)

@alexcrichton
Copy link
Member

I am in favor of this change, although I find @erickt's comment a little disturbing. I would have hoped that LLVM would realize for values without a destructor that it can optimize away the value inside the loop, and then it'd optimize the whole loop away.

@thestinger, could you look into why LLVM isn't optimizing it away? I would like to avoid the needs_drop conditional if possible, and if we can invert some condition or something like that to get LLVM to optimize it away that would be nice.

@thestinger
Copy link
Contributor Author

@alexcrichton: The substantial change already landed for this and this is just some cleanup not really changing much.

@thestinger
Copy link
Contributor Author

@erickt: I'm okay with doing that, but would rather fix whatever is wrong with our code generation. It could be caused by the stupid way we handle self combined with the lack of any aliasing information.

@bors bors closed this Dec 17, 2013
@bors bors merged commit 664c9af into rust-lang:master Dec 17, 2013
@thestinger thestinger deleted the iter branch January 15, 2014 08:42
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 30, 2023
Add WebAssembly to allowed idents

Closes rust-lang#10998
changelog: [`doc_markdown`]: No longer lints WebAssembly
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.

4 participants