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

Use inner iterator may_have_side_effect for Cloned #56538

Merged
merged 2 commits into from
Dec 6, 2018

Conversation

KamilaBorowska
Copy link
Contributor

@KamilaBorowska KamilaBorowska commented Dec 5, 2018

Previous implementation wasn't correct, as an inner iterator could have had side effects. Noticed by @bluss in #56534.

@rust-highfive
Copy link
Collaborator

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 5, 2018
src/libcore/iter/mod.rs Outdated Show resolved Hide resolved
@KamilaBorowska
Copy link
Contributor Author

Oops, modified this in wrong place, let me fix that.

Previous implementation wasn't correct, as an inner iterator
could have had side effects.
@bluss
Copy link
Member

bluss commented Dec 5, 2018

Playground snippet to show the bug, that it silences sideeffects in map.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=9d308367741f5ca7cb80ee50ae9a8168

I think we want to fix this because we don't want our use of specialization to be user-visible or affect sideeffectfulness of iterations.

@KamilaBorowska
Copy link
Contributor Author

I will try adding a test case for this bug when I get home.

@bluss
Copy link
Member

bluss commented Dec 5, 2018

@bors r+ rollup

thanks!

@bors
Copy link
Contributor

bors commented Dec 5, 2018

📌 Commit a964307 has been approved by bluss

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 5, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Dec 5, 2018
Use inner iterator may_have_side_effect for Cloned

Previous implementation wasn't correct, as an inner iterator could have had side effects. Noticed by @bluss in rust-lang#56534.
pietroalbini added a commit to pietroalbini/rust that referenced this pull request Dec 5, 2018
Use inner iterator may_have_side_effect for Cloned

Previous implementation wasn't correct, as an inner iterator could have had side effects. Noticed by @bluss in rust-lang#56534.
bors added a commit that referenced this pull request Dec 5, 2018
Rollup of 15 pull requests

Successful merges:

 - #51753 (Document `From` implementations)
 - #55563 (Improve no result found sentence in doc search)
 - #55987 (Add Weak.ptr_eq)
 - #56119 (Utilize `?` instead of `return None`.)
 - #56372 (Refer to the second borrow as the "second borrow" in E0501.rs)
 - #56388 (More MIR borrow check cleanup)
 - #56424 (Mention raw-ident syntax)
 - #56452 (Remove redundant clones)
 - #56456 (Handle existential types in dead code analysis)
 - #56466 (data_structures: remove tuple_slice)
 - #56476 (Fix invalid line number match)
 - #56497 (cleanup: remove static lifetimes from consts in libstd)
 - #56498 (Fix line numbers display)
 - #56523 (Added a bare-bones eslint config (removing jslint))
 - #56538 (Use inner iterator may_have_side_effect for Cloned)

Failed merges:

r? @ghost
@bors bors merged commit a964307 into rust-lang:master Dec 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants