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

Adding the has_item function to the iterator trait. #127604

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c068fdc
implemented the contains feature
yonikremer Jul 8, 2024
c143b42
the function contains can now get every type that can be borrowed as …
yonikremer Jul 9, 2024
7931998
added tests for contain
yonikremer Jul 9, 2024
fd40ecb
formatted the code
yonikremer Jul 9, 2024
5372e4b
fixed failing tests
yonikremer Jul 9, 2024
82d481d
using the feature contains
yonikremer Jul 10, 2024
15d6c9f
added the tests for the "contains" feature
yonikremer Jul 10, 2024
48616ce
added a test for checking that the function can check if a String is …
yonikremer Jul 10, 2024
7ba6c5e
moved tests for the contains feature to the test file of the Iterator…
yonikremer Jul 10, 2024
b69db01
changed function signature according to the rust library team request
yonikremer Jul 10, 2024
3751b5d
removed tests from function doc string
yonikremer Jul 10, 2024
7ebf719
getting arguments that implement arg == Self::Item and not argument w…
yonikremer Jul 10, 2024
ac8d1e3
rust formatting: deleted 2 empty lines
yonikremer Jul 10, 2024
cd839b8
removed unneeded feature specification
yonikremer Jul 10, 2024
66c55b2
renamed contain to has_item
yonikremer Jul 11, 2024
b67ee44
renamed feature from has_item to iter_has_item
yonikremer Jul 11, 2024
1ba1ca7
added a test for the short-circuiting behaviour
yonikremer Jul 11, 2024
934a194
fixed test_short_circuiting
yonikremer Jul 11, 2024
3e3ccd8
fixed test_short_circuiting
yonikremer Jul 11, 2024
bc20afd
renamed feature and function back to contains
yonikremer Jul 11, 2024
8a7c12b
changed excepted compiler suggestion. Now the compiler is suggesting …
yonikremer Jul 11, 2024
dac6cd7
deleted a test that uses the function contains of a range
yonikremer Jul 11, 2024
2a32a8c
deleted a test that uses the function contains of a range
yonikremer Jul 11, 2024
ba5b980
removed the contains feature from library/core/Cargo.toml
yonikremer Jul 11, 2024
e70effd
made the doc tests small and simple
yonikremer Jul 11, 2024
82b6b10
made the docs more consistent with similar methods of other types
yonikremer Jul 11, 2024
2f72670
fixed typos
yonikremer Jul 11, 2024
fb98975
improved doc example
yonikremer Jul 12, 2024
d8fc920
Merge remote-tracking branch 'origin/master'
yonikremer Jul 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion library/core/src/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1327,7 +1327,7 @@ pub trait Iterator {
/// `take(n)` yields elements until `n` elements are yielded or the end of
/// the iterator is reached (whichever happens first).
/// The returned iterator is a prefix of length `n` if the original iterator
/// contains at least `n` elements, otherwise it contains all of the
/// contains at least `n` elements, otherwise it containss all of the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// contains at least `n` elements, otherwise it containss all of the
/// contains at least `n` elements, otherwise it contains all of the

/// (fewer than `n`) elements of the original iterator.
///
/// # Examples
Expand Down Expand Up @@ -4060,6 +4060,40 @@ pub trait Iterator {
{
unreachable!("Always specialized");
}

/// Returns `true` if the iterator contains a value.
///
/// 'contains' is short-circuiting; in other words, it will stop processing
/// as soon as the function finds the item in the `Iterator`.
///
/// Performance:
/// This method checks the whole iterator, which takes O(n) time.
/// If the iterator is sorted, or a hash map please use the appropriate method instead.
Comment on lines +4069 to +4071
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Performance:
/// This method checks the whole iterator, which takes O(n) time.
/// If the iterator is sorted, or a hash map please use the appropriate method instead.
/// This method checks the whole iterator, which is O(n). If the iterator is a sorted
/// slice, [`binary_search`](slice::binary_search) may be faster. If this is an iterator
/// on collections that have a `.contains()` or `.contains_key()` method (such as
/// [`HashMap`] or [`BTreeSet`]), using those methods directly will be faster.

We can be more specific about what the appropriate methods are here. No section heading needed, I think rustdoc would have just merged Performance: into the next paragraph anyway.

You can check the rendering with ./x doc library/core --open

///
/// Example:
/// ```
/// #![feature(iter_contains)]
/// assert!([1, 2, 3].iter().map(|&x| x * 3).contains(&6));
/// assert!(![1, 2, 3].iter().contains(&4));
/// // You can check if a String is in a string slice iterator.
/// assert!(["a", "b", "c"].iter().contains(&"b".to_owned()));
/// // You can also check if a String iterator contains a string slice.
/// assert!(["a".to_owned(), "b".to_owned(), "c".to_owned()].iter().contains(&"b"));
/// ```
///
#[unstable(feature = "iter_contains", reason = "new API", issue = "127494")]
fn contains<Q: ?Sized>(&mut self, item: Q) -> bool
where
Q: PartialEq<Self::Item>,
Self: Sized,
{
for element in self {
if item == element {
return true;
}
}
return false;
}
}

/// Compares two iterators element-wise using the given function.
Expand Down
1 change: 1 addition & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@
#![feature(ip)]
#![feature(is_ascii_octdigit)]
#![feature(isqrt)]
#![feature(iter_contains)]
#![feature(link_cfg)]
#![feature(offset_of_enum)]
#![feature(offset_of_nested)]
Expand Down
59 changes: 59 additions & 0 deletions library/core/tests/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,65 @@ fn test_next_chunk() {
let mut it = std::iter::repeat_with(|| panic!());
assert_eq!(it.next_chunk::<0>().unwrap(), []);
}
#[test]
fn test_happy_path_item_not_in_iterator() {
assert!(![1i32, 2i32, 3i32].iter().contains(&4i32));
}

#[test]
fn test_edge_case_handling_none_values() {
assert!([Some(2i32), Option::<i32>::None].iter().contains(&None));
assert!([Some(2i32), Option::<i32>::None].iter().contains(&Some(2i32)));
}

#[test]
fn test_edge_case_handling_empty_iterator() {
assert!(!Vec::<i32>::new().iter().contains(&1i32));
}

#[test]
fn test_edge_case_handling_iterator_with_duplicates() {
assert!([1i32, 2i32, 2i32, 3i32].iter().contains(&2i32));
}

#[test]
/// Tests that short-circuiting works correctly when using `contains`
/// When you run the function, it should move the iterator forward after the first appearance of the item
fn test_short_circuiting() {
let vector: Vec<i32> = vec![1i32, 2i32, 3i32, 1i32, 1i32];
let mut iterator = vector.into_iter();
assert!(iterator.contains(1i32));
assert_eq!(iterator.next(), Some(2));
assert!(!iterator.contains(4i32));
assert_eq!(iterator.next(), None);
}

#[test]
fn test_edge_case_handling_iterator_with_custom_struct() {
Comment on lines +619 to +653
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: since these tests only check .contains(), they should have test_contains in the name

#[derive(PartialEq)]
struct Item {
value: i32,
}
assert!([Item { value: 1i32 }, Item { value: 2i32 }].iter().contains(&Item { value: 2i32 }));
}

#[test]
fn test_str_iterator_contains_string() {
assert!(["a", "b", "c"].iter().contains(&"b".to_owned()));
assert!(!["a", "b", "c"].iter().contains(&"d".to_owned()));
}

#[test]
fn test_str_iterator_contains_string_slice() {
assert!(["a", "b", "c"].iter().contains(&"b"));
assert!(!["a", "b", "c"].iter().contains(&"d"));
}

#[test]
fn test_string_iterator_contains_str_slice() {
assert!(["a".to_owned(), "b".to_owned(), "c".to_owned()].iter().contains(&"b"));
assert!(!["a".to_owned(), "b".to_owned(), "c".to_owned()].iter().contains(&"d"));
}

// just tests by whether or not this compiles
fn _empty_impl_all_auto_traits<T>() {
Expand Down
1 change: 1 addition & 0 deletions library/core/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
#![feature(const_mut_refs)]
#![feature(const_pin)]
#![feature(const_waker)]
#![feature(iter_contains)]
#![feature(never_type)]
#![feature(unwrap_infallible)]
#![feature(pointer_is_aligned_to)]
Expand Down
11 changes: 11 additions & 0 deletions src/tools/rust-analyzer/crates/ide-db/src/generated/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6074,6 +6074,17 @@ The tracking issue for this feature is: None.

------------------------

"##,
},
Lint {
label: "contains",
description: r##"# `contains`

The tracking issue for this feature is: [#94047]

[#94047]: https://github.com/rust-lang/rust/issues/94047


Comment on lines +6077 to +6087
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is autogenerated so no need for adding this (its also adding it one slot too late)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yonikremer see this comment too, you can revert this file

The `rustc` compiler has certain pluggable operations, that is,
functionality that isn't hard-coded into the language, but is
implemented in libraries, with a special marker to tell the compiler
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/suggestions/deref-path-method.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
fn main() {
let vec = Vec::new();
Vec::contains(&vec, &0);
//~^ ERROR no function or associated item named `contains` found for struct `Vec<_, _>` in the current scope
//~^ ERROR `Vec<_, _>` is not an iterator [E0599]
//~| HELP the function `contains` is implemented on `[_]`
}
Comment on lines 1 to 6
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit of an unfortunate suggestion change for an unstable method. This was added in #100302, cc @compiler-errors since this will no longer test what it was intended to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls file a bug, we probably can filter by stability or sth idk

7 changes: 5 additions & 2 deletions tests/ui/suggestions/deref-path-method.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0599]: no function or associated item named `contains` found for struct `Vec<_, _>` in the current scope
error[E0599]: `Vec<_, _>` is not an iterator
--> $DIR/deref-path-method.rs:3:10
|
LL | Vec::contains(&vec, &0);
| ^^^^^^^^ function or associated item not found in `Vec<_, _>`
| ^^^^^^^^ `Vec<_, _>` is not an iterator
|
note: if you're trying to build a new `Vec<_, _>` consider using one of the following associated functions:
Vec::<T>::new
Expand All @@ -11,6 +11,9 @@ note: if you're trying to build a new `Vec<_, _>` consider using one of the foll
Vec::<T>::from_raw_parts
and 4 others
--> $SRC_DIR/alloc/src/vec/mod.rs:LL:COL
= note: the following trait bounds were not satisfied:
`Vec<_, _>: Iterator`
which is required by `&mut Vec<_, _>: Iterator`
help: the function `contains` is implemented on `[_]`
|
LL | <[_]>::contains(&vec, &0);
Expand Down
Loading