Skip to content

add default FromIterator for types with Default and Extend trait #143996

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

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

HernandoR
Copy link

@HernandoR HernandoR commented Jul 16, 2025

relating to #58659
r? @hkBst

@rustbot
Copy link
Collaborator

rustbot commented Jul 16, 2025

Failed to set assignee to hkBst: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc F-autodiff `#![feature(autodiff)]` S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 16, 2025
@HernandoR HernandoR force-pushed the lz/feat/default_from_iterator branch from e85cd44 to d8d2e82 Compare July 16, 2025 01:00
@HernandoR
Copy link
Author

Sorry, bad source branch. The labels may be incorrect, shall I start a new pr?

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@HernandoR HernandoR force-pushed the lz/feat/default_from_iterator branch from 854316a to c0faef8 Compare July 16, 2025 04:24
@rust-log-analyzer

This comment has been minimized.

…d trait

Signed-off-by: HernandoR <lzhen.dev@outlook.com>
@HernandoR HernandoR force-pushed the lz/feat/default_from_iterator branch from c0faef8 to 1653767 Compare July 16, 2025 05:41
@rust-log-analyzer

This comment has been minimized.

@@ -152,6 +152,17 @@ pub trait FromIterator<A>: Sized {
fn from_iter<T: IntoIterator<Item = A>>(iter: T) -> Self;
}

// 为同时实现 Default 和 Extend 的类型实现 FromIterator
#[unstable(feature = "from_iter_default", issue = "58659")]
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately unstable impl's are not supported (yet?), so you should remove this line and its accompanying feature.

Copy link
Member

Choose a reason for hiding this comment

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

Should be #[stable(feature = "from_iter_default", since = "CURRENT_RUSTC_VERSION")], and I was probably wrong about removing the feature.

@rust-log-analyzer

This comment has been minimized.

Co-authored-by: Marijn Schouten <hkBst@users.noreply.github.com>
@rust-log-analyzer

This comment has been minimized.

Signed-off-by: HernandoR <lzhen.dev@outlook.com>
@rust-log-analyzer

This comment has been minimized.

Signed-off-by: HernandoR <lzhen.dev@outlook.com>
@rust-log-analyzer

This comment has been minimized.

@HernandoR HernandoR marked this pull request as ready for review July 16, 2025 12:08
@rustbot
Copy link
Collaborator

rustbot commented Jul 16, 2025

Failed to set assignee to hkBst: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 16, 2025
@HernandoR HernandoR marked this pull request as draft July 16, 2025 12:08
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 16, 2025
@HernandoR HernandoR requested a review from hkBst July 16, 2025 12:08
@HernandoR
Copy link
Author

Suppose there is a tuple type (Vec<i32>, Vec<&str>) that satisfies both:

The constraints of the generic implementation: (Vec<i32>, Vec<&str>): Default + Extend<(i32, &str)> (since each element Vec implements Default and Extend).
The constraints of the tuple implementation: Vec<i32>: Default + Extend<i32> and Vec<&str>: Default + Extend<&str>.

In this case, when from_iter is called, the compiler would need to choose between:

The generic implementation: with T = (Vec<i32>, Vec<&str>) and A = (i32, &str).
The tuple implementation: the specialized implementation generated specifically for this tuple.

In theory, the tuple implementation is more specific. However, the compiler cannot confirm the safety of this specialization—because both implementations rely on the core logic of Default::default() and Extend::extend, whose behavior is defined by external traits (Default/Extend). The compiler cannot guarantee that their behavior in the tuple scenario will be consistent with the generic scenario.

Signed-off-by: HernandoR <lzhen.dev@outlook.com>
@HernandoR
Copy link
Author

i could not tell if feature = "specialization" is enabled in my local test, how to make ./x build and test for a nightly version of rust lib? i.e. with specialization feature enabled

@rust-log-analyzer
Copy link
Collaborator

The job pr-check-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] build_script_build test:false 0.334
    Checking la-arena v0.3.1
[RUSTC-TIMING] la_arena test:false 0.328
   Compiling zerovec-derive v0.11.1
error[E0277]: a value of type `(Vec<N>, Vec<rowan::SyntaxNode<RustLanguage>>)` cannot be built from an iterator over elements of type `(N, rowan::SyntaxNode<RustLanguage>)`
    --> src/tools/rust-analyzer/crates/syntax/src/ast/syntax_factory/constructors.rs:1332:10
     |
1332 |         .collect()
     |          ^^^^^^^ value of type `(Vec<N>, Vec<rowan::SyntaxNode<RustLanguage>>)` cannot be built from `std::iter::Iterator<Item=(N, rowan::SyntaxNode<RustLanguage>)>`
     |
     = help: the trait `FromIterator<(N, rowan::SyntaxNode<RustLanguage>)>` is not implemented for `(Vec<N>, Vec<rowan::SyntaxNode<RustLanguage>>)`
     = help: the trait `FromIterator<()>` is implemented for `()`
note: the method call chain might not have had the expected associated types
    --> src/tools/rust-analyzer/crates/syntax/src/ast/syntax_factory/constructors.rs:1328:10
     |
1326 |       input
     |       ----- this expression has type `impl IntoIterator<Item = N>`
1327 |           .into_iter()
     |            ----------- `Iterator::Item` is `N` here
1328 |           .map(|it| {
     |  __________^
1329 | |             let syntax = it.syntax().clone();
1330 | |             (it, syntax)
1331 | |         })
     | |__________^ `Iterator::Item` changed to `(N, SyntaxNode<RustLanguage>)` here
note: required by a bound in `collect`
    --> /checkout/library/core/src/iter/traits/iterator.rs:2014:19
     |
2014 |     fn collect<B: FromIterator<Self::Item>>(self) -> B
     |                   ^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Iterator::collect`

[RUSTC-TIMING] zerovec_derive test:false 2.712
   Compiling displaydoc v0.2.5
[RUSTC-TIMING] displaydoc test:false 1.236
   Compiling query-group-macro v0.0.0 (/checkout/src/tools/rust-analyzer/crates/query-group-macro)

@hkBst
Copy link
Member

hkBst commented Jul 16, 2025

@HernandoR I'm not sure what's going wrong here. Combining these impl seems to work fine on the playground: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=1f1f887ddb3205ed2a3d488d7f05955f.

I'm reading up on specialization, but it's probably a good idea to ask for help on Zulip.

@QuineDot
Copy link

As a blanket implementation, this would be a major breaking change.

I can have this today, and this change conflicts with it.

#[derive(Default)]
struct S(usize);

impl<I> Extend<I> for S {
    fn extend<T: IntoIterator<Item = I>>(&mut self, iter: T) {
        self.0 += iter.into_iter().count();
    }
}

impl<I> FromIterator<I> for S {
    fn from_iter<T: IntoIterator<Item = I>>(iter: T) -> Self {
        Self(
            iter.into_iter().count() + 100
        )
    }
}

@HernandoR
Copy link
Author

@QuineDot I think that's exactly what feature specialization trying to solve: Providing a default implement for a trait that would be overridden if any more "specific" implement is provided

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc F-autodiff `#![feature(autodiff)]` S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants