owned this note
owned this note
Published
Linked with GitHub
# Libs-API Meeting 2024-01-23
###### tags: `Libs Meetings` `Minutes`
**Meeting Link**: https://meet.jit.si/rust-libs-meeting-crxoz2at8hiccp7b3ixf89qgxfymlbwr
**Attendees**: Amanieu, David, Josh Triplett, Mara, Chris Denton, The 8472, TC, Urgau
## Agenda
- Triage
- Anything else?
## Triage
### FCPs
17 rust-lang/rust T-libs-api FCPs
- merge rust.tf/80437 *Tracking Issue for \`box\_into\_inner\`* - (1 checkboxes left)
- merge rust.tf/52331 *Correcting Path::components on Redox* - (5 checkboxes left)
- merge rust.tf/82901 *Tracking Issue for \`Option::get\_or\_insert\_default\`* - (2 checkboxes left)
- merge rust.tf/83871 *Tracking Issue for CharIndices::offset function* - (3 checkboxes left)
- merge rust.tf/98934 *Add \`Option::take\_if\`* - (3 checkboxes left)
- merge rust.tf/115386 *PartialEq, PartialOrd: update and synchronize handling of transitive chains* - (3 checkboxes left)
- merge rust.tf/106943 *Implement DoubleEnded and ExactSize for Take\<Repeat\> and Take\<RepeatWith\>* - (3 checkboxes left)
- merge rust.tf/96992 *Tracking Issue for \`waker\_getters\`* - (3 checkboxes left)
- merge rust.tf/99262 *Tracking Issue for \`io\_error\_downcast\`* - (3 checkboxes left)
- merge rust.tf/62726 *Tracking issue for io\_slice\_advance* - (3 checkboxes left)
- merge rust.tf/106418 *Implement \`PartialOrd\` and \`Ord\` for \`Discriminant\`* - (0 checkboxes left)
- merge rust.tf/109402 *Implement owned ops for \`HashSet\` and \`BTreeSet\`* - (4 checkboxes left)
- merge rust.tf/116113 * Generalize \`{Rc,Arc}::make\_mut()\` to unsized types.* - (4 checkboxes left)
- merge rust.tf/113833 *\`std::error::Error\` \-\> Trait Implementations: lifetimes consistency improvement* - (2 checkboxes left)
- merge rust.tf/115974 *Split core's PanicInfo and std's PanicInfo* - (3 checkboxes left)
- merge rust.tf/117468 *Stabilize Wasm relaxed SIMD* - (5 checkboxes left)
- merge rust.tf/101196 *Tracking Issue for \`Ready::into\_inner()\`* - (2 checkboxes left)
[dtolnay (2)](https://rfcbot.rs/fcp/dtolnay), [joshtriplett (13)](https://rfcbot.rs/fcp/joshtriplett), [BurntSushi (9)](https://rfcbot.rs/fcp/BurntSushi), [Amanieu (9)](https://rfcbot.rs/fcp/Amanieu), [m-ou-se (9)](https://rfcbot.rs/fcp/m-ou-se)
### (nominated) rust.tf/119131 *Tracking Issue for \`hint::assert\_unchecked\`*
Name good?
David: Name seems fine
The 8472: It's consistent
David: This is better than `assume`, more honest about how it is used. No expectations what the compiler does with this information.
Amanieu: So, FCP?
Mara: Agree it's consistent.
Amanieu replied.
### (nominated) rust.tf/120219 *core: implement Add for ascii::Char; drop ascii::Char::digit*
Mara: I don't mind adding Add, but we shouldn't remove .digit().
The 8472: The motivation for removing `digit` is because char::digit has a different signature: it takes a radix. but that's a weird reason for *removing* it.
David: from the author of the Add PR:
https://github.com/rust-lang/rust/pull/118963#issuecomment-1902111388
"from_digit is over-engineered"
Mara: This `Add` for `ascii::Char` implementation wraps on overflow in release mode. That's not as logical/useful as that behaviour on our regular integer types.
Mara: If you want to do arithmetic on ascii chars, just convert to u8 first. and you can use `b'0'` syntax for that too.
Amanieu: so, just close this?
Mara: I think so, with an explanation.
David: Isn't wrapping what we always do for integers in release?
Mara: Yes, but this is a 7-bit integer. machine instructions don't result in this behaviour like they do for u8/u16/etc.
Mara: Feels a bit like having `Add<int>` for NonNull that would just turn 0 into 1 implicitly in release mode. Surprising and not very useful.
Josh: It's just a 7-bit integer though, not a non-something
The 8472: masking is just one instruction
David: Yeah, the point is to avoid the extra code size of the panic on every addition. but one instruction can be fine.
David: The from impls seem fine. Maybe as a separate change.
Mara: from impls seem fine. removing `digit` is not fine. not convinced we should have a (wrapping) `Add`.
David: We should also discuss whether we want `u8 + ascii::Char` instead of just `ascii::Char + u8`
Mara: We don't have `+` for `u8` and `u16`, so why would we have it for `u8` and `u7`?
Josh: `.offset()` instead of `+`?
Mara: I think `+` is fine to use for this operation, but i don't think it should be symmetrical, which would just add expectations on mixing integer types.
David: Agree. In `ascii::Char + u8`, it's obvious to me that the Output type would be ascii::Char. In `u8 + ascii::Char`, the correct choice of Output type is less obvious
Josh: I think it's fine to not see ascii::Char as u7 but as a special type. i'd like it to be symmetrical if we use `+`.
Amanieu: char doesn't have Add.
Mara: `char` has more validity requirements. it has multiple valid ranges.
Amanieu: still, i think ascii::Char shouldn't use the Add operator.
Mara: Char implements Step, right? Then you can do `('0'..).nth(10)`
Amanieu: conclusion?
Josh: Seems clear we're not removing `digit`, but might want to add a `radix` argument.
Amanieu: we're happy with the From impls
Amanieu: we're conflicted on the Add trait.
1) not have it at all
2) have .offset or something like that
3) add Add as proposed
4) add Add asymetrically, with ascii::Char only on the left.
Amanieu: prefer 1, would accept 2
Josh: opposed to 4. like 2 and 3
The 8472: like option 2, would accept 4
David: prefer 4, would accept 1, would abstain for 2 or 3 if everyone else agrees
Mara: 1 or 4. wouldn't accept `offset`, but `wrapping_add`/`checked_add` would be fine with me.
David: We want people to use ascii::Char whenever they are working with ascii. If we require conversion back and forth then people will just avoid it.
Josh: Right, then people will just use u8 *everywhere*, like they would in C.
Mara: I think that's part of a larger problem where we can't make things like NonZero and Wrapping/Saturating as ergonomic as we want.
Josh: I think even if we had pattern types today, we wouldn't replace ascii::Char with `u8 is 0..=127`.
Mara: Do people avoid `char` today?
Amanieu: yes, but mostly because it's 32 bit.
The 8472: u8 already has a lot of char/ascii methods.
Amanieu: should we defer and try to get anything else done?
Josh to reply.
### (nominated) rust.tf/rfc3509 *RFC: Include \`Future\` and \`IntoFuture\` in the 2024 prelude*
Mara: Seems fine?
Josh: RFCbot merge this and handle consensus that way?
David: How about Future but not IntoFuture?
Josh: it's commonly called in wrappers etc. and unlikely it'd cause issues/conflicts.
David: compelling argument.
Josh: I'll submit a suggestion and open FCP.
### (waiting on team) rust.tf/117925 *Handle out of memory errors in io:Read::read\_to\_end()*
The 8472: don't we already do this somewhere else?
TC: Previously:
https://github.com/rust-lang/rust/pull/70012
https://github.com/rust-lang/rust/pull/84612
https://github.com/rust-lang/rust/pull/116570
Mara: Thios PR is a continuation of https://github.com/rust-lang/rust/pull/116570
that had an FCP, but a concern about a documentation guarantee
Amanieu: so, do we guarantee that it will *not* panic when out of memory?
Mara: i don't think we should promise things about fallible allocations just yet.
Amanieu: I'm happy with this PR.
Amanieu: FCP?
Mara: Went through FCP last time, and this doesn't really need an api FCP because it doesn't really change the API. Maybe just r+'ing is fine.
Assigned to Amanieu.
### (waiting on team) rust.tf/119991 *Reject infinitely\-sized reads from io::Repeat*
The 8472: Should return OutOfMemory, see previous item.
Josh: Agree.
Amanieu: Unsupported is also reasonable.
Josh: Also fine, prefer OutOfMemory.
Amanieu: Prefer Unsupported.
Mara: Hm, OutOfMemory kind-of implies that having more memory would have fixed the issue.
The 8472: You would have gotten OutOfMemory without this 'optimization'.
Josh: Seems like nobody objects tore turning an error.
Josh: Does anyone object to Unsupported?
Amanieu: Let's go with OutOfMemory to make everyone happy; i don't care.
The 8472 to reply.
### (waiting on team) rust.tf/119550 *Rename \`AsyncIterator\` back to \`Stream\`, introduce an AFIT\-based \`AsyncIterator\` trait*
Not waiting on us.
### (stalled change proposal) rust.tf/libs177 *Add slice::parts*
Mara: Seems too specific, maybe just not add it?
Amanieu: Put it in IterTools?
The 8472: Left a comment before. For big things you want something more advanced.
Josh: The difference with chunks is that instead of putting all the 'unevenness' in the last chunk, you spread those over multiple chunks.
Amanieu: better off using rayon
Mara: you might just want to use (thread_index..).skip(n_threads)
Josh: sounds like consensus is not accepting this.
Mara: We can just reply that we're not convinced. they can always re-open with more convincing arguments.
Josh to reply.
### Waker/RawWaker: https://github.com/rust-lang/rust/issues/96992#issuecomment-1902780274
David: I'm unable to rule out the alternatives.
Mara: If it's useful to have a separate type for not-validated pointers, use alternative 1. otherwise alternative 2.
Mara: We can do 1 now, and 2 later.
Josh: +1 for just having public fields
Amanieu: and .as_raw() to read the fields from Waker?
David: yes
David: boats says all three options are fine too.
David: boats: "No strong opinion on those alternatives, I would say do whatever seems most std-like to do. The alternatives mean committing to more, which should be fine, but also having to use these getters in the kind of code that needs this isn't the worst thing in the world. So I don't see a reason to care a lot."
David: I'll cancel the FCP, and propose public fields on RawWaker instead.
### (new change proposal) rust.tf/libs328 *Add \`Option::merge\`*
Mara: Left a comment with a question on use case of the `add` example.
Amanieu: I'll comment something.
### (new change proposal) rust.tf/libs325 *Add macro \`static\_assert\` to perform compile\-time assertion checking*
Josh: do we need to wait on the language feature, or can we just add the macro even if this can be written as `const { assert!() }` later?
Mara: I think the lang team discussion on `const { assert!() }` could influence our decision here. e.g. name it `const_assert!()`.
Josh: I'll make sure the lang team discusses this.
### (new change proposal) rust.tf/libs322 *\`AssertThreadSafe\` (name TBD) – a more general API for lifting conservative \`!Send\` or \`!Sync\` implementations*
Too complicated to go into in this meeting now.
### (new change proposal) rust.tf/libs327 *Add Wrapping/Saturating::from\_mut reference conversions*
Mara: This can easily 'explode': you could also safely do this for slices, Vec, and many other containers. Discussing with niko what to do with these repr(transparent) 'transmuting' problems..
Mara: Feels like this ACP solves a tiny part of a much bigger problem, but not necessarily against.
Josh: If we had a language feature for such a safe transmute, would we deprecate methods like this?
Amanieu: at the least we want something with a machine applicable suggestion.
Mara: If these methods proposed here have a clear use case that don't also require tons more (like the related slice methods), let's accept.
Amanieu: use case is if Wrapping appears in a a funciton signature?
Mara: I believe Wrapping should *never* appear in a function signature. (Similar to how `mut` is not part of the signature in `fn a(mut x: i32)`.)
### (stalled change proposal) rust.tf/libs155 *Arbitrary alternate flags in \`std::fmt::Formatter\`*
Mara: don't like this.
Amanieu: isn't this better handled by wrapper types?
Josh: Same for the other flags, but we're here now.
Mara: I can write a reply explaining why I think the proposal is a bad idea.
### (stalled change proposal) rust.tf/libs124 *Integrate \`Error\` trait with panic interfaces*
(Some discussion about how we may want to defer this until after considering splitting PanicInfo into core+std portions.)
Mara offers to present the state of panic, in next week's meeting.
### bool::implies https://github.com/rust-lang/libs-team/issues/188
Mara: The mathematician in me likes this, but every other part hates it. I think there's a small set of people for who this is very readable, and a bigger group of people for who this is confusing.
Josh: +1
TC: I'm in the group of people who like it.
Mara: Could be a community crate with an extension trait specifically for those who like these math terms.
TC: If we had methods for all of the logical operators in std, you could rely on using these in method chains, so more people would learn and know them. Having just one, as here, makes it less likely to carry its weight. Since we don't have the rest of these, that could be a reason to close this.
The 8472: We have the same with option/result/iterator, each has their own 'language' that you need to learn. Niche methods aren't easy to remember.
Mara: Yeah, `map` people remember because it is common, but `implies` is not common. It's more clear to just use the operators.
TC: On the other hand, the name can document clearer what the intention is, rather than having to work backward that by `!a || b`, the author did mean an `implies` relationship.
Mara: That's what comments are for. We wouldn't add another version of `map` with only a different name, just to document a different use case.
Consensus: close.
Amanieu replying.
### (stalled change proposal) rust.tf/libs111 *Restructure ptr\_metadata to minimal support*
### (stalled change proposal) rust.tf/libs164 *Add methods for use cases that \`align\_to\` does not cover*
### (new change proposal) rust.tf/libs323 *Add \`ptr::fn\_addr\_eq\` to compare functions pointers.*