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

Minor cleanups now that #9629 is fixed #10930

Merged
merged 3 commits into from
Dec 21, 2013

Conversation

DaGenix
Copy link

@DaGenix DaGenix commented Dec 12, 2013

3 minor clean-ups now that #9629 is fixed:

  • Update MutChunkIter to remove the remainder that existed just to allow the size_hint() method to be implemented. This is no longer necessary since we can just access the length of the slice directly.
  • Update MutSplitIterator to address the FIXME in its size_hint() method. This method was only partially implemented due to the issue. Also, implement a minor optimization in the case that its the last iteration.
  • Update ByRef iterator to implement the size_hint() method.

I noticed that MutSplitIterator returns an empty slice if called on an empty slice. I don't know if this is intended or not, but I left the finished field in-place to preserve this behavior.

@TeXitoi @blake2-ppc

@DaGenix
Copy link
Author

DaGenix commented Dec 12, 2013

Ug. Looks like it opened this a little bit early since I just realized that this won't work until the next snapshot. I will rebase this when the next snapshot is made.

@alexcrichton
Copy link
Member

Hopefully soon! (#10919)

@TeXitoi
Copy link
Contributor

TeXitoi commented Dec 12, 2013

The behavior of the MutSplitIterator on empty slice is the same as SplitIterator. I'm OK with this, no remarks. Thanks!

@DaGenix
Copy link
Author

DaGenix commented Dec 14, 2013

Looks like the snapshot from #10919 doesn't include the fix for #9629. I'll wait for the next snapshot.

@DaGenix
Copy link
Author

DaGenix commented Dec 14, 2013

@TeXitoi makes sense. Still sounds like there is some inconsistent behavior, though. I'm not really sure whats correct, however. I opened #10962 to discuss.

Palmer Cox added 3 commits December 20, 2013 20:40
This field is no longer necessary now that rust-lang#9629 is fixed since we can just
access the length of the remaining slice directly.
Update the next() method to just return self.v in the case that we've reached
the last element that the iterator will yield. This produces equivalent
behavior as before, but without the cost of updating the field.

Update the size_hint() method to return a better hint now that rust-lang#9629 is fixed.
@DaGenix
Copy link
Author

DaGenix commented Dec 21, 2013

The merge failed since there was an unused variable which I missed removing. I've rebased to get rid of that variable which should fix that issue.

bors added a commit that referenced this pull request Dec 21, 2013
…crichton

3 minor clean-ups now that #9629 is fixed:

* Update MutChunkIter to remove the ```remainder``` that existed just to allow the size_hint() method to be implemented. This is no longer necessary since we can just access the length of the slice directly.
* Update MutSplitIterator to address the FIXME in its size_hint() method. This method was only partially implemented due to the issue. Also, implement a minor optimization in the case that its the last iteration.
* Update ByRef iterator to implement the size_hint() method.

I noticed that MutSplitIterator returns an empty slice if called on an empty slice. I don't know if this is intended or not, but I left the ```finished``` field in-place to preserve this behavior.

@TeXitoi @blake2-ppc
@bors bors closed this Dec 21, 2013
@bors bors merged commit 765bc90 into rust-lang:master Dec 21, 2013
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 30, 2023
[`redundant_closure_call`]: handle nested closures

Fixes rust-lang#9956.

This ended up being a much larger change than I'd thought, and I ended up having to pretty much rewrite it as a late lint pass, because it needs access to certain things that I don't think are available in early lint passes (e.g. getting the parent expr). I think this'll be required to fi-x rust-lang#10922 anyway, so this is probably fine.
(edit: had to write "fi-x" because "fix" makes github think that this PR fixes it, which it doesn't 😅 )

Previously, it would suggest changing `(|| || 42)()()` to `|| 42()`, which is a type error (it needs parens: `(|| 42)()`). In my opinion, though, the suggested fix should have really been `42`, so that's what this PR changes.

changelog: [`redundant_closure_call`]: handle nested closures and rewrite as a late lint pass
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