Skip to content

Use len instead of size_hint where appropiate #34425

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
Jun 24, 2016

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Jun 23, 2016

This makes it clearer that we're not just looking for a lower bound but
rather know that the iterator is an ExactSizeIterator.

This makes it clearer that we're not just looking for a lower bound but
rather know that the iterator is an `ExactSizeIterator`.
@rust-highfive
Copy link
Contributor

r? @alexcrichton

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

@nagisa
Copy link
Member

nagisa commented Jun 23, 2016

Doesn’t len have an assertion which size_hint doesn’t? I guess I’m being too paranoid, but there certainly must be some extra overhead which warrants some benchmarks… e.g. how does the performance of iterating though CharIndices change?

@tbu-
Copy link
Contributor Author

tbu- commented Jun 23, 2016

If that were the case, we should rather modify len than use size_hint in places where len would be more appropiate.

But it looks like LLVM can see right through it:

use std::slice;

pub fn slice_len(iter: &slice::Iter<u8>) -> usize {
    iter.len()
}

pub fn slice_len2(iter: &slice::Iter<u8>) -> usize {
    iter.size_hint().0
}

produces the same bytecode

; ModuleID = 'a.0.rs'
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

%"2.std::slice::Iter<u8>" = type { i8*, i8*, %"2.std::marker::PhantomData<&'static u8>" }
%"2.std::marker::PhantomData<&'static u8>" = type {}

; Function Attrs: norecurse readonly uwtable
define i64 @_ZN1a9slice_len17hc2890fcf572cc394E(%"2.std::slice::Iter<u8>"* noalias nocapture readonly dereferenceable(16)) unnamed_addr #0 {
entry-block:
  %1 = getelementptr inbounds %"2.std::slice::Iter<u8>", %"2.std::slice::Iter<u8>"* %0, i64 0, i32 1
  %2 = bitcast i8** %1 to i64*
  %3 = load i64, i64* %2, align 8, !alias.scope !0, !noalias !5
  %4 = bitcast %"2.std::slice::Iter<u8>"* %0 to i64*
  %5 = load i64, i64* %4, align 8, !alias.scope !0, !noalias !5
  %6 = sub i64 %3, %5
  ret i64 %6
}

; Function Attrs: norecurse nounwind readonly uwtable
define i64 @_ZN1a10slice_len217h415029022c569338E(%"2.std::slice::Iter<u8>"* noalias nocapture readonly dereferenceable(16)) unnamed_addr #1 {
entry-block:
  %1 = getelementptr inbounds %"2.std::slice::Iter<u8>", %"2.std::slice::Iter<u8>"* %0, i64 0, i32 1
  %2 = bitcast i8** %1 to i64*
  %3 = load i64, i64* %2, align 8, !alias.scope !7, !noalias !10
  %4 = bitcast %"2.std::slice::Iter<u8>"* %0 to i64*
  %5 = load i64, i64* %4, align 8, !alias.scope !7, !noalias !10
  %6 = sub i64 %3, %5
  ret i64 %6
}

attributes #0 = { norecurse readonly uwtable }
attributes #1 = { norecurse nounwind readonly uwtable }

!0 = !{!1, !3}
!1 = distinct !{!1, !2, !"_ZN79_$LT$std..slice..Iter$LT$$u27$a$C$$u20$T$GT$$u20$as$u20$std..iter..Iterator$GT$9size_hint17h39fecfdf3007b42aE: argument 1"}
!2 = distinct !{!2, !"_ZN79_$LT$std..slice..Iter$LT$$u27$a$C$$u20$T$GT$$u20$as$u20$std..iter..Iterator$GT$9size_hint17h39fecfdf3007b42aE"}
!3 = distinct !{!3, !4, !"_ZN4core4iter6traits17ExactSizeIterator3len17hb31ac8a36a76a083E: argument 0"}
!4 = distinct !{!4, !"_ZN4core4iter6traits17ExactSizeIterator3len17hb31ac8a36a76a083E"}
!5 = !{!6}
!6 = distinct !{!6, !2, !"_ZN79_$LT$std..slice..Iter$LT$$u27$a$C$$u20$T$GT$$u20$as$u20$std..iter..Iterator$GT$9size_hint17h39fecfdf3007b42aE: argument 0"}
!7 = !{!8}
!8 = distinct !{!8, !9, !"_ZN79_$LT$std..slice..Iter$LT$$u27$a$C$$u20$T$GT$$u20$as$u20$std..iter..Iterator$GT$9size_hint17h39fecfdf3007b42aE: argument 1"}
!9 = distinct !{!9, !"_ZN79_$LT$std..slice..Iter$LT$$u27$a$C$$u20$T$GT$$u20$as$u20$std..iter..Iterator$GT$9size_hint17h39fecfdf3007b42aE"}
!10 = !{!11}
!11 = distinct !{!11, !9, !"_ZN79_$LT$std..slice..Iter$LT$$u27$a$C$$u20$T$GT$$u20$as$u20$std..iter..Iterator$GT$9size_hint17h39fecfdf3007b42aE: argument 0"}

@alexcrichton
Copy link
Member

Yes most of these look like it's trivial for LLVM to see through as the size_hint method is just returning (a, Some(a)) which is easy to cause the assertion that (Some(low) == hi) to get optimized away. Looks fine to me.

@bors: r+ 8ff5c43

@bors
Copy link
Collaborator

bors commented Jun 23, 2016

⌛ Testing commit 8ff5c43 with merge ad07411...

@bors
Copy link
Collaborator

bors commented Jun 23, 2016

💔 Test failed - auto-mac-64-opt

@alexcrichton
Copy link
Member

@bors: retry

On Thu, Jun 23, 2016 at 4:00 PM, bors [email protected] wrote:

💔 Test failed - auto-mac-64-opt
http://buildbot.rust-lang.org/builders/auto-mac-64-opt/builds/9566


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#34425 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAD95LMBFxGwiGVYuRHs0pT2m2nXRbgNks5qOxACgaJpZM4I8qoV
.

@bors
Copy link
Collaborator

bors commented Jun 24, 2016

⌛ Testing commit 8ff5c43 with merge 90943a5...

bors added a commit that referenced this pull request Jun 24, 2016
Use `len` instead of `size_hint` where appropiate

This makes it clearer that we're not just looking for a lower bound but
rather know that the iterator is an `ExactSizeIterator`.
@alexcrichton
Copy link
Member

@bors: retry force clean

  • restarted buildbot

@bors
Copy link
Collaborator

bors commented Jun 24, 2016

⌛ Testing commit 8ff5c43 with merge 4b89deb...

bors added a commit that referenced this pull request Jun 24, 2016
Use `len` instead of `size_hint` where appropiate

This makes it clearer that we're not just looking for a lower bound but
rather know that the iterator is an `ExactSizeIterator`.
@bors bors merged commit 8ff5c43 into rust-lang:master Jun 24, 2016
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.

5 participants