--- date: 2024-06-04 url: https://hackmd.io/2B71dGVyT22UCfJAb40JWg --- # Libs-API Meeting 2024-06-04 ###### tags: `Libs Meetings` `Minutes` **Meeting Link**: https://meet.jit.si/rust-libs-meeting-crxoz2at8hiccp7b3ixf89qgxfymlbwr **Attendees**: Amanieu, Josh, TC, Chris Denton, The8472, eholk ## Agenda - Triage - Anything else? ## Triage ### FCPs 19 rust-lang/rust T-libs-api FCPs - merge rust.tf/80437 *Tracking Issue for \`box\_into\_inner\`* - (1 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/102012 *Tracking Issue for \`const\_waker\`* - (3 checkboxes left) - merge rust.tf/106943 *Implement DoubleEnded and ExactSize for Take\<Repeat\> and Take\<RepeatWith\>* - (3 checkboxes left) - merge rust.tf/62726 *Tracking issue for io\_slice\_advance* - (3 checkboxes left) - merge rust.tf/109402 *Implement owned ops for \`HashSet\` and \`BTreeSet\`* - (3 checkboxes left) - merge rust.tf/116113 * Generalize \`{Rc,Arc}::make\_mut()\` to unsized types.* - (3 checkboxes left) - merge rust.tf/115974 *Split core's PanicInfo and std's PanicInfo* - (2 checkboxes left) - merge rust.tf/101196 *Tracking Issue for \`Ready::into\_inner()\`* - (1 checkboxes left) - merge rust.tf/119131 *Tracking Issue for \`hint::assert\_unchecked\`* - (1 checkboxes left) - merge rust.tf/106418 *Implement \`PartialOrd\` and \`Ord\` for \`Discriminant\`* - (2 checkboxes left) - close rust.tf/56167 *Tracking issue for HashMap::raw\_entry* - (3 checkboxes left) - merge rust.tf/113219 *Tracking Issue for \`const\_cstr\_from\_ptr\`* - (5 checkboxes left) - merge rust.tf/55132 *Tracking issue for thread::Builder::spawn\_unchecked* - (3 checkboxes left) - merge rust.tf/123723 *Make \`std::os::tvos\`, \`std::os::visionos\` and \`std::os::watchos\` public* - (4 checkboxes left) - merge rust.tf/125112 *Document behavior of \`create\_dir\_all\` wrt. empty path* - (4 checkboxes left) - merge rust.tf/117618 *Tracking Issue for \`duration\_abs\_diff\`* - (3 checkboxes left) - merge rust.tf/53485 *Tracking issue for RFC 2351, "Add \`is\_sorted\` to the standard library"* - (4 checkboxes left) [scottmcm (1)](https://rfcbot.rs/fcp/scottmcm), [dtolnay (2)](https://rfcbot.rs/fcp/dtolnay), [Amanieu (7)](https://rfcbot.rs/fcp/Amanieu), [yaahc (2)](https://rfcbot.rs/fcp/yaahc), [pnkfelix (1)](https://rfcbot.rs/fcp/pnkfelix), [joshtriplett (16)](https://rfcbot.rs/fcp/joshtriplett), [BurntSushi (10)](https://rfcbot.rs/fcp/BurntSushi), [m-ou-se (11)](https://rfcbot.rs/fcp/m-ou-se), [tmandry (1)](https://rfcbot.rs/fcp/tmandry), [nikomatsakis (2)](https://rfcbot.rs/fcp/nikomatsakis) ### (nominated) rust.tf/125751 *Add \`new\_range\_api\` for RFC 3550* Unresolved questions: https://github.com/rust-lang/rust/issues/125687 The8472: Doesn't adding the `Iterator` methods to these re-raise the issue of these types being `Copy`? Amanieu: This was to make the migration easier. But I agree, I'd prefer to not these at all. Josh: At least we should take that in a second pass. We shouldn't tie it to the initial edition addition of `Range`. Amanieu: These are only useful for the migration. The8472: This seems like an interesting question: > * Should `RangeFrom` even implement `IntoIterator`, or should it require an explicit `.iter()` call? Using it as an iterator [can be a footgun](https://github.com/rust-lang/libs-team/issues/304), usually people want `start..=MAX` instead. Also, it is inconsistent with `RangeTo`, which doesn't implement `IntoIterator` either. But, it's extremely useful for `it.zip(1..)` Amanieu: My feeling is that all of the range types should support `IntoIterator`. It would start at the lowest value. The8472: That doesn't work for `Step`. Amanieu: That's unstable, so we can change it. Josh: There's an interesting case here for e.g. `..3` that can come up due to type inference. Josh: In terms of what we can do over the edition, the fact that `rev` is the second-most used method, could and should we make `for i in 20..0` do the thing we think it should do? Amanieu: That would add a branch in the implementation, which could inhibit optimizations. Josh: It's only at the start of the loop, not in the loop. Amanieu: I'm 80% sure this would break optimization. Josh: I'd like to check this. eholk: If these are literals, this is OK. But if this is `for m..n`, this is less intuitive. Josh: Agree, I withdraw that suggestion. Amanieu: Let's take this question: > * The exact module paths and type names > * Should the new types live at `std::ops::range::` instead? > * `IterRange`, `IterRangeInclusive` or just `Iter`, `IterInclusive`? Or `RangeIter`, `RangeInclusiveIter`, ...? The8472/Amanieu: Nobody cares about the names of these. Josh: I don't care which either. The person implementing it should just pick. Amanieu: `core::range` is fine. I'm happy with how it's implemented. Amanieu: Next question: > * Should other range-related items (like `RangeBounds`) also be moved under the `range` module? Amanieu: Yes. I think this makes sense. We'll probably want to move the step stuff into `range`. Amanieu: Next: > * Should `RangeFrom` even implement `IntoIterator`, or should it require an explicit `.iter()` call? Using it as an iterator [can be a footgun](https://github.com/rust-lang/libs-team/issues/304), usually people want `start..=MAX` instead. Also, it is inconsistent with `RangeTo`, which doesn't implement `IntoIterator` either. But, it's extremely useful for `it.zip(1..)` The8472: I think it's a conservative start to keep it out. Amanieu: The issue is the overflowing behavior. The8472: If you know that your input is bounded, those edge cases are irrelevant. The8472: ...see zip. Josh: Zip is a good argument that `RangeFrom` should implement `IntoIterator`. Amanieu: There is an ACP to change this behavior. The8472: There are iterators like for bigint that can't necessarily set the end value. Amanieu: But we should change things so that the end value can be emitted. all: Agreed. Amanieu: Next: > * Should there be a way to get an iterator that modifies the range in place, rather than taking the range by value? That would allow things like `range.by_ref().next()`. Amanieu: I've never needed to do that. The8472: Does that even work? Amanieu: I think we can defer this until someone specifically asks for it. > * Should there be an infallible conversion from legacy to new `RangeInclusive`? Amanieu: And in this case, how do we handle the exhaustive case? Amanieu: The current implementation has a `From` impl and debug asserts. The8472: The debug asserts don't do anything in `std`. The8472: We could convert to the iterator. Josh: I don't think that'll be what people are looking for. The idea here is that you can `.into()` on the old range types to get one of the new ones. The8472: Wo do say that `From` impls shouldn't panic. Josh: We do, but the general rule is that panics are for programming errors. In this case, the old range types are being used for both iteration and a representation for the syntax. The panic behavior is right for at least one of these. The8472: Maybe we should warn on this. Josh: That would defeat the purpose. We shouldn't be naive about what warninings mean. That's us telling people not to do this. The8472: Adding a new panic path seems questionable. Josh: I think this is going to be really uncommon. There's no literal that you could write to hit this. You'd have to step it manually at least once, then call `.into()` on that. The8472: `Into` would be the wrong thing there anyway since ownership would be taken. So OK. Amanieu: To summarize: - The inherit methods, we don't want them for now. - We're mostly happy with the module paths and type names. So, `core::range` it is. - We're happy to move `RangeBounds` and `Step` into the `range` module. Josh: I can't help but notice that there's a strong step function in how much some of these methods are used. There are a lot of uses of `map` and `rev`. I'm tempted. Amanieu: The behavior would be different though. The8472: We could give them a different name. Amanieu: I'd be happy with `for_each` but probably not `map`. Josh: I'm looking at the pattern that people commonly invoke. There's a link to a search. That pattern seems reasonable. The8472: But you can fix that by adding `.iter()`. Josh: Probably `.into_iter()`, but yes. I wonder what value we're getting by making people's code more verbose. If people see that as their first impression, people may ask what the point was. The8472: That's a marketing concern though. There are real motivations here. Amanieu: I'd be strongly against adding `map` and `rev` as they are. I think it'd be easier to add `.into_iter()`. Amanieu: I'd like to have an `.iter()` here even though it will do the same thing as `.into_iter()`, to save the characters. Josh: There is the caveat that, normally, `.iter()` gives you references. This would be different. Are we OK with that? Amanieu: Yes. There is precedent for that. In MPSC, the `Receiver` has one that returns objects by value. Josh: It seems it will be intuitive for ranges of numbers. It seems less intuitive for others. But since we don't expose `Step`, that seems OK. The8472: We could think about the conditional `Copy` bounds here. Amanieu: Ranges right now work with non-`Copy` types. I don't think there's any reason to require `Copy`. The8472: OK, but then would `.iter()` take `self` and rely on the `Copy` behavior, or would it take `&self` and `.clone()`? Amanieu: The current implementation is based on cloning. Josh: Having the inherit `.iter()` method be a `&self` method seems helpful. It would let you reuse the range for multiple iterations. Amanieu: But we're talking about the niche cache of non-`Copy` ranges. Josh: The distinction is only one that applies to non-`Clone` ranges. I'm tempted to have this rely on `Step` which is unstable. Amanieu: We'd still need to decide whether to take `self` or `&self`. Josh: Someone could always call `.into_iter()`. Josh: Setting aside the details, I do think it's valuable to provide a `.iter()` method. Amanieu: We should have it, but we need to think about the details, e.g. what it should take. So it'd be the equivalent of `.clone().into_iter()`? The8472: Yes. Amanieu: To summarize: - The inherit methods, we don't want them for now. - We're mostly happy with the module paths and type names. So, `core::range` it is. - We're happy to move `RangeBounds` and `Step` into the `range` module. - We're happy to have `From` impl on `RangeInclusive` and we're happy to have it panic. - `RangeFrom` is a tricky one; the iterator would need to have a bool; would that inhibit optimization? - We don't want any `.by_ref()` on `Range`. Amanieu: I'll summarize the discussion on the issue. ### (nominated) rust.tf/125937 *Remove lock guarantee from \`std::env::{set\_var, remove\_var}\`* TC: tbu sets out the heart of the problem here: > On Linux with glibc, I think that this comment at most pushes people to think that their multi-threaded use case of `set_var`/`remove_var` is safe, even when they most likely **cannot guarantee that in any way**. Any standard library function might read the environment tomorrow, if Rust or glibc decides so. The8472: The guarantees could be useful on other platforms. Josh: We have a lot of caveats. Just not stating what we do doesn't seem like an improvement. Amanieu: I don't feel strongly about this. I don't think changing a few words here makes any difference in practice. I'm happy either way. Josh: It sounds that we don't have a consensus on changing the status quo. Amanieu: As mentioned, I'm happy either way. ### (nominated) rust.tf/rfc3642 *\[RFC\] Thread spawn hook (inheriting thread locals)* Skipped until Mara is available. ### (waiting on team) rust.tf/119550 *Rename \`AsyncIterator\` back to \`Stream\`, introduce an AFIT\-based \`AsyncIterator\` trait* Not waiting on us. ### (waiting on team) rust.tf/123723 *Make \`std::os::tvos\`, \`std::os::visionos\` and \`std::os::watchos\` public* Let's defer until David is back. ### (waiting on team) rust.tf/125112 *Document behavior of \`create\_dir\_all\` wrt. empty path* Josh: This is about documenting the current behavior of this edge case. Amanieu: Sounds reasonable; I've checked my box. Josh: Likewise. ### (new change proposal) rust.tf/libs382 *Add \`substr\_range\`, \`elem\_offset\`, and \`subslice\_range\` methods* - Panic instead of returning Option - Panic on ZST ### (new change proposal) rust.tf/libs383 *Add \`FRAC\_1\_SQRT\_2PI\` constant* Defer until we hear from Mara. ### (new change proposal) rust.tf/libs386 *mixed\_integer\_ops extension* Josh: This seems perfectly reasonable. Amanieu: I think this is fine to add. Josh: Especially as the implementation is not trivial and has a corner case, we should add this. Amanieu: Let's approve it. Josh: I'll reply to it. ### (new change proposal) rust.tf/libs387 *\`std::env::{user\_config\_path, user\_cache\_path}\`: per\-user application paths* Josh: I'm in favor, in general, of having a way to get user directories in the standard library. But I'm worried about taking only a small subset of the `dirs` or `directories` crate. We may have to take a larger subset; the complexity may not be reducible. Chris: Go exposes these two; not sure how that's worked out for them. Amanieu: On the one hand, I'd like to leave this to a crate. But on the other, I could see people hand coding it rather than taking the import. Josh: I'd love to do this, but the proposal here does not seem sufficient. Amanieu: Let's defer it then. Josh: We could reach out to the maintainers of the current crates and see whether they're interested in bringing a proposal. ### (new change proposal) rust.tf/libs388 *Extend std::fs::Permissions on Windows* (Discussion.) ### (stalled change proposal) rust.tf/libs269 *Add \`try\_into\_boxed\` (and potentially \`into\_boxed\_inner\`) for \`Rc\<T\>\`/\`Arc\<T\>\` where \`T: ?Sized\`* Ask author whether `UniqueArc` solves their use case. ### (stalled change proposal) rust.tf/libs257 *Implement \`From\<&'a &'static str\>\` for \`Arguments\<'a\>\`* Deferred until Mara is available. ### (stalled change proposal) rust.tf/libs111 *Restructure ptr\_metadata to minimal support* ### (stalled change proposal) rust.tf/libs231 *ACP: Add const fn TypeId::matches for comparing type ids in consts* ### (stalled change proposal) rust.tf/libs163 *array: add unsafe methods to convert slices to array references* Accepted + safe checked versions of these ### Gut check on adding `Pin` + `pin!` to prelude TC: We have `Unpin` in the prelude but not `Pin` or `pin!`. These are used commonly in writing certain kinds of async code. I need these all the time, and every time I `use` them, I feel like they should be in the prelude. These are fundamental language items at this point. If someone used either `Pin` or `pin!` to mean something else, they'd be "doing it wrong". As a gut check, what's the appetite for a proposal to add these? Amanieu: I don't think I've ever once used either of these in code. Josh: Pinning is unfortunate, but until we come up with something better, these are fundamental, and are used in writing certain kinds of code. Our standard shouldn't be that these are useful everywhere, but instead that they're useful in some places and that the downside is low. Here, it does seem the downside is low, as these shouldn't be used to mean anything else. TC: So the next step is an ACP or PR? Josh/Amanieu: We accept either ACPs or PRs for nominations. Probably here, just doing a PR is better. Josh: Note that this will be a substantial PR is terms of what will have to be changed, e.g. in tests. scottmcm (after the meeting): I wonder if we should have area preludes. Pin for async; Path for CLI; etc. Where often things that are "need these all the time" are completely unused in other domains. --- _Generated by [fully-automatic-rust-libs-team-triage-meeting-agenda-generator](https://github.com/rust-lang/libs-team/tree/main/tools/agenda-generator)_