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

libstd: Use std::array::from_fn to implement Default for arrays larger than 32 elements. #124163

Closed
wants to merge 1 commit into from

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Apr 19, 2024

Much like #74060 did for every other trait.

This avoids tools like rust-bindgen having to painfully work around this limitations, sometimes incorrectly like in
rust-lang/rust-bindgen#2803

@rustbot
Copy link
Collaborator

rustbot commented Apr 19, 2024

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler 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 Apr 19, 2024
@emilio
Copy link
Contributor Author

emilio commented Apr 19, 2024

cc some of the people involved in #74060: @withoutboats @nikomatsakis @SimonSapin

@rust-log-analyzer

This comment has been minimized.

…r than 32 elements.

Much like rust-lang#74060 did for every other trait.

This avoids tools like rust-bindgen having to painfully work around this
limitations, sometimes incorrectly like in
rust-lang/rust-bindgen#2803
fn default() -> [T; $n] { [] }
}
};
#[stable(feature = "default_array_lib", since = "1.79.0")]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if this is the right thing to do annotation wise.

@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot)

Comment on lines -431 to -433
// The Default impls cannot be done with const generics because `[T; 0]` doesn't
// require Default to be implemented, and having different impl blocks for
// different numbers isn't supported yet.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this resolved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh... I totally misread that, ugh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I could fix this by using a conditional impl using generic_const_exprs, but that causes a bunch of other code in libcore to fail to build, see #124167

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any other clever way of getting this to work out of curiosity? (Not familiar with the specialisation features that are available in nightly)

Copy link
Member

@Nilstrieb Nilstrieb Apr 19, 2024

Choose a reason for hiding this comment

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

generic_const_exprs is very much forbidden from being used on the standard library, no matter this issue. it's way too broken for that.

@jhpratt
Copy link
Member

jhpratt commented Apr 20, 2024

Closing for the reason stated. There is no way that this can currently be done in the standard library, as the necessary features are simply not mature enough. Personally, the last time I tried to implement Default (circa August 2023) for arrays of all sizes, the trivial demo code I wrote wouldn't even compile.

@jhpratt jhpratt closed this Apr 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler 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.

None yet

6 participants