# Libs-API Meeting 2023-07-11

###### tags: `Libs Meetings` `Minutes`

**Meeting Link**: https://meet.jit.si/rust-libs-meeting-crxoz2at8hiccp7b3ixf89qgxfymlbwr
**Attendees**: Amanieu, David, Mara, The 8472, Chris Denton, Urgau, TC

## Agenda

- Triage
- Anything else?

## Triage

### FCPs

10 rust-lang/rust T-libs-api FCPs

- merge rust.tf/88581 *Tracking Issue for \`int\_roundings\`* - (1 checkboxes left)
- merge rust.tf/80437 *Tracking Issue for \`box\_into\_inner\`* - (1 checkboxes left)
- merge rust.tf/91946 *Tracking Issue for \`io::Error::other\`* - (3 checkboxes left)
- merge rust.tf/98461 *Document lack of panic safety guarantees of \`Clone::clone\_from\`* - (0 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* - (4 checkboxes left)
- merge rust.tf/99262 *Tracking Issue for \`io\_error\_downcast\`* - (4 checkboxes left)
- merge rust.tf/106655 *Tracking Issue for \`#!\[feature(offset\_of)\]\`* - (0 checkboxes left)
- merge rust.tf/107587 *Mark \`std\` integral modules as deprecated (\`std::u32\`, \`std::i16\`, etc.)* - (4 checkboxes left)

[joshtriplett (3)](https://rfcbot.rs/fcp/joshtriplett), [yaahc (4)](https://rfcbot.rs/fcp/yaahc), [Amanieu (3)](https://rfcbot.rs/fcp/Amanieu), [dtolnay (3)](https://rfcbot.rs/fcp/dtolnay), [m-ou-se (5)](https://rfcbot.rs/fcp/m-ou-se), [BurntSushi (6)](https://rfcbot.rs/fcp/BurntSushi)

### (nominated) rust.tf/98704 *Implement From\<OwnedFd/Handle\> for ChildStdin/out/err object*

Left nominated last time?

Last week's notes:

> Mara to review and start FCP if it looks okay.

### (nominated) rust.tf/113283 *Type annotations needed in assertion*

Type inferrence issue after the PR we merged recently.

It has already been reverted.

(Nothing to discuss. Just nominated for awareness.)

### (waiting on team) rust.tf/112858 *Update Android system definitions and add riscv\-linux\-android as tier 3 target*

The libs part was split off and that part has been merged. So this PR is no longer libs related.

### (new change proposal) rust.tf/libs234 *ACP: Add \`Cow::clear\` methods*

Discussed a month ago. Responded, but no response for author.

Close?

### (new change proposal) rust.tf/libs235 *ACP: Extended logic for IP networks*

Discussed last time. Mara still needs to reply.

### (new change proposal) rust.tf/libs236 *Implement \`PartialEq\`, \`Eq\`, \`PartialOrd\`, \`Ord\`, \`Hash\` for unix \`SocketAddr\`*

Was already accepted. Close.

### (new change proposal) rust.tf/libs238 *ACP: \`NonNanFNN\` and \`FiniteFNN\` float wrappers*

Discussion seems to have died down. Nobody seems in favor.

Doesn't seem worth it. This doesn't satisfy for many use cases.

David: The optimization use case might not work, but the 'float that is Ord' use case is still useful.

Amanieu: That can live in an external crate.

David: True.

David: Let's steer people towards a lang feature for specifying niches.

David to respond. (And link the relevant lang RFC?)

### (new change proposal) rust.tf/libs244 *add wrapping\_offset\_from*

Doesn't seem necessary anymore: https://github.com/rust-lang/rust/issues/92512#issuecomment-1620562580

> Looking at the current code in libcore, I think slice iterators can handle ZST without ever doing a pointer subtraction in the ZST case, so I think the only concrete motivation we had for this issue vaporized? Only the non-ZST path uses ptr_sub but that is fine even during CTFE as all pointers are in-bounds.

The 8472 to reply

### rust.tf/libs115 ACP: Range::cmp_scalar; comparison (less/equal/greater) to a primitive of the Range

`binary_search_by` example seemed compelling. from two meetings ago:

> There's more to discuss than we can fit in this meeting, but do we want something like this at all?
>
> Mara: Yes
>
> Josh: Yes
>
> .. *silence*

Five questions in the ACP:

> - Does the name cmp_scalar make sense? To me it kind of does, but I see no precedent with calling stuff "scalars" in the stdlib. I wonder if there are better alternatives.

Mara: Bad name. Calling the elements 'scalars' isn't right in all cases.

Amanieu: 'point'?

Mara: 'element'? 'value'?

Amanieu: Not 'element', it's not a collection.

> - Ord vs PartialOrd? With contains, PartialOrd makes sense, but with a proper ordering returned by this API, I think that Ord is the only sensible choice.

David: Using PartialOrd means returning an Option. I prefer this to take Ord and returning Ordering.

The 8472: What about invalid ranges, with start higher than end?

Mara: Those are empty ranges. is_empty returns true

David: We can define it to always return Less in those cases.

The 8472: Or panic.

David: That requires an extra check.

[tangent]

Mara: The argument order / return value feels 'swapped'. Given 0..10 and 100, it gives 'less', but the value is greater than the range.

David: 0..10 < 100, so makes sense.

Mara: Sure, but it quickly gets confusing. I'd see this as asking a question about where the value is, not where the range is.

David: Maybe 'cmp_relative_to' or something to make it clearer.

[back to Ord/PartialOrd]

David: Prefer returning Ordering without Option. Require Ord, and don't check for end < start. (Just return Less or Greater in that case.)

David: We could have an additional `partial_cmp_scalar` for PartialOrd that returns Option.

Mara: Happy to start with just the Ord version, adding PartialOrd variant only if someone needs it.

The 8472: A bad PartialOrd impl can result in a value being less than the start and greater than the end.

David: Not worth checking for broken PartialOrd impls.

Mara: Agreed.

> - The method currently takes the scalar by copy. It might make sense to take it by reference to support textual ranges? That worsens the ergonomics for numbers though.

Mara: .contains() takes a reference, so this should too.

David: Agree.

> - Does it make sense to make it more generic (to support &str vs String comparisons, for example)? That might worsen the ergonomics because it makes type inference harder.

Mara: I think yes, because contains() also does that.

Amanieu: Yup.

> - Does it make sense to actually just implement PartialOrd\<T\> for Range\<T\>, instead of having this as a separate method? Can we consider this kind of comparison "canonical" enough to be able to provide a standard implementation?

[..]

Amanieu: There is a better way to do binary search over ranges. Just binary search over only the ends (or starts) of the ranges, and then at the end check that range only.

The 8472: That removes the motivation for cmp_scalar.

Amanieu: This way isn't trivial though.

The 8472: Still, that doesn't mean we should add cmp_scalar for this use case.

Amanieu to reply.

### (stalled change proposal) rust.tf/libs139 *Add back \`BorrowedBuf::filled\_mut\`*

Mara: Seems fine.

David: You can already do .clear() and re-fill, so you could already mutate it anyway.

Amanieu: There is lots of discussion in the PR: https://github.com/rust-lang/rust/pull/103754

Mara: The ACP was created after all that discussion.

Mara approved the ACP and the PR.

### (stalled change proposal) rust.tf/libs104 *std::io: vectored reads with uninitialized memory (Read::read\_buf\_vec)*

Amanieu: This also solves the `advance_slices` issue. That api is unstable and not great. This gives a `self` type to implement that on.

Mara: It's a large API, but it's pretty self-contained. I imagine this all just goes into its own file, so it's low risk to just accept the ACP and see where it goes.

Mara: `from_slice` takes a &mut [u8]? is that an error?

Amanieu: You could use it to write MaybeUninit::uninit() into the [u8].

Mara: Does that actually do anything?

The 8472: Maybe.

The 8472: Can this replace BorrowedBuf? Special case for 1?

Amanieu: Not sure if that makes sense. We can ask the author.

Mara: It's annoying to use for that use case, because this type has a nother layer of indirection. Need to store that &mut/IoSlice somewhere.

Mara: Might be nice if it was better integrated with IoSlice.

Mara: Anyway, still happy to accept this and see where it goes.

The 8472 to reply.

### (stalled change proposal) rust.tf/libs128 *Add \`inner\` and \`into\_inner\` methods to iterator adapters*

The 8472: This is an issue for some of our optimizaitons/specializations that assume no external access to the underlying iterator.

Amanieu: The use case seems to be specific to Peekable. Should we just accept adding it to that?

Mara: Sure.

Amanieu: Oh, also Filter.

Not in favor of doing this for all adapters, but there might be specific adapters for which this is more useful than others.

David to reply.

### (stalled change proposal) rust.tf/libs124 *Integrate \`Error\` trait with panic interfaces*

Mara: This adds a &dyn Error to the PanicInfo. Interesting question whether that error should then also be the payload() or if the payload() is separate. That's why I'm trying to remove the `Send` bound from payload() so it can return the Error.

Mara: Also related to Result::unwrap(). calling that on a Err(E) with E: Error should result in this kind of PanicInfo, but that's tricky.

Amanieu: Yeah that seems impossible.

Mara: Nothing is impossible if you Edition hard enough. ^^'

Amanieu: (:

Mara: I think adding this as unstable is fine, but the error should be a special case of payload, such that .payload() returns the error.

That is blocked on https://github.com/rust-lang/rust/pull/110799

Amanieu: That PR is fine. Error in panic seems... sure.. but it's a shame it can't work with unwrap.

Mara: Specialization?

The 8472: We can't specialize on lifetimes. So we can't make SomeError<'static> do something different than SomeError<'a>.

Amanieu: We would need new APIs on `Result` for this to be useful. Do we still want to go forward without them?

Amanieu: Either reject and ask for better solution to `unwrap`/`expect`. Or accept to allow experimentation.

David: Don't see how this would improve Rust without unwrap/expect.

Amanieu: Changing `unwrap` over edition is too big a breaking change.

David: Not eager, panics could be nicer but this information is already present in the panic message. Panics are there to help you fix a bug, this doesn't really help.

Amanieu to reply.

### (stalled change proposal) rust.tf/libs111 *Restructure ptr\_metadata to minimal support*