# Libs-API Meeting 2023-08-08 ###### tags: `Libs Meetings` `Minutes` **Meeting Link**: https://meet.jit.si/rust-libs-meeting-crxoz2at8hiccp7b3ixf89qgxfymlbwr **Attendees**: Amanieu, David, Josh Triplett, Mara, The 8472, Chris Denton, TC, Urgau ## Agenda - Triage - Anything else? ## Triage ### FCPs 1 rust-lang/rfcs T-libs-api FCPs - merge rust.tf/rfc3453 *Add \`f16\` and \`f128\` float types* - (6 checkboxes left) 13 rust-lang/rust T-libs-api FCPs - 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.)* - (3 checkboxes left) - merge rust.tf/62726 *Tracking issue for io\_slice\_advance* - (3 checkboxes left) - merge rust.tf/114041 *Implement \`From\<\[T; N\]\>\` for \`Rc\<\[T\]\>\` and \`Arc\<\[T\]\>\`* - (3 checkboxes left) - merge rust.tf/114065 *\`impl TryFrom\<char\> for u16\`* - (4 checkboxes left) - merge rust.tf/85528 *Implement iterator specialization traits on more adapters* - (5 checkboxes left) [m-ou-se (9)](https://rfcbot.rs/fcp/m-ou-se), [joshtriplett (6)](https://rfcbot.rs/fcp/joshtriplett), [BurntSushi (10)](https://rfcbot.rs/fcp/BurntSushi), [dtolnay (4)](https://rfcbot.rs/fcp/dtolnay), [the8472 (1)](https://rfcbot.rs/fcp/the8472), [Amanieu (5)](https://rfcbot.rs/fcp/Amanieu) ### (nominated) rust.tf/92122 *Tracking issue for thread local Cell methods* Discussed and started FCP last week. Removed label. ### (nominated) rust.tf/101155 *Map EINPROGRESS to io::ErrorKind::WouldBlock* Discussed last week. Concluded we don't want to merge with EAGAIN. No consensus on adding a new ErrorKind of this yet. Close PR now that we reopened the ACP? Amanieu: Looks like we're stuck, because we lack visibility in how this would impact real-world code. Mara: A while back when we discussed new ErrorKinds, we added a mechanism for adding them as unstable. Now that we have that, we can just add this as unstable? JoshT: Having it as unstable for a while seems perfectly reasonable. Amanieu: The point of ErrorKind is to write OS-independent error handling. Last time we discussed how EINPROGRESS has different meanings on Windows and Unix. So is this helpful? Mara: Yeah.. Not sure if it's helpful to have an ErrorKind that's too platform specific. JoshT: Is that a blocker for adding as unstable or a blocker for stabilizing? Mara: Blocker for stabilizing. Adding as unstable seems fine. But if we already know that we won't stabilize anyway, we shouldn't add it as unstable. Amanieu: I'm reading the man page for connect(2). It seems that it depends on the socket type whether it returns EINPROGRESS or EAGAIN. Which makes me think it makes sense to merge them? Amanieu: Unix sockets return EAGAIN, others return EINPROGRESS.. Mara: I don't think we can solve this design in this meeting. Can we comment on the ACP, asking someone to collect the information? JoshT: If we're only on Unix, it seems to have a clear meaning. But it's not clear how it extends to Windows. Do we need to ask for a Windows expert? Mara: Yes, but also on Unix it doesn't seem entirely clear (e.g. Amanieu's example before). Mara to reply to this. ### (nominated) rust.tf/109049 *split\_array: remove runtime panics* Discussed last time. Conclusion was that we can't make it a purely compile-time error due to the `if else` use case. A lint would be a good solution in today's Rust. David: returning an Option seems wrong. It should work like split_at(). JoshT: Code that wants an option can use an iterator instead. Mara: Agreed that this should panic. David: Downside is that an Option is easier to convert to panic than the other way around. Mara: It should behave the same as Index and split_at, etc. (Panic.) The 8472: We miss the tools to do this properly at compile time. (const if? dead code path elimination before the check?) David to reply. ### (nominated) rust.tf/111544 *Tracking Issue for os\_str\_bytes* Amanieu: Naming question. Mara: I like the way it is. JoshT: Likewise. Amanieu: We're returning bytes that are a superset of UTF-8. What do we call this? Mara: During our in-person meetup we called it 'os str bytes', which seemed fine. Amanieu: Sure. I don't think it really matters. Amanieu: having 'os str' in the method name seems a bit redundant though. Mara: 'os str bytes' is about the [u8], not about the OsStr. David: I prefer 'encoded'. When someone calls this, they need to make sure the encoding matches. That's less clear otherewise. JoshT: I care more about stabilizing this than about the name. Mara: 'encoded' makes it sound like encoding/decoding is going on, instead of a O(1) conversion. The 8472: We're exposing the internal representation. How do we encode that in the name? David: 'from internal byte representation' The 8472: 'os str bytes' can sound like the native os encoding. Mara: I prefer something without 'encoded', but wouldn't block on that. David: Do you prefer 'encoded' or 'internal representation'? Mara: Between those i prefer 'encoded', because we might define the encoding at some later point in time, such that it is no longer internal. David: Or 'platform independent' something? Since you deal with it the same on all platforms. Mara: If we define the encoding on specific platforms, users might do platform-specific things with it. Amanieu: 'encoded'? David: +1 Mara: Fine. It gives us the possibility to give stronger guarantees in the future if we want to. Amanieu to reply and to start FCP. JoshT: Let's also start FCP. ### (nominated) rust.tf/112387 *Don't panic in ceil\_char\_boundary* Mara: Why does floor_char_boundary not panic?? Amanieu: Good question. Mara: My initial reaction is that both these methods should panic for out-of-bounds JoshT: Same. David: Seems useful if it doesn't panic. Like if you want to show the first ~100 bytes, you can just call it with 100. Mara: is_char_boundary returns false, doesn't panic. so after the string we have an infinite stream of 'not char boundaries'. so following that logic, floor_char_boundary not panicking beyond the end and returning .len() makes sense. JoshT: yeah that's consistent, but ugh. Mara: I've seen this used in rustc as a 'index sanitizing' function. Given an 'untrusted' index as input, these functions return a non-panicking index. So then it's useful that it never panicks. David: Valid. I'm in favor of this. Amanieu: with this PR the `ceil` function rounds 'down' to str.len() though. Mara: Yes, but that's the highest index we can give that wouldn't panic in split_at. David: I like that perspective, of using these functions as sanitizing the input to guarantee split_at doesn't panic. Amanieu: ok Mara to comment and ~~FCP~~ The 8472: Doesn't need FCP. Unstable. Mara to comment and review. :) ### (nominated) rust.tf/114384 *Soft deprecate Sink* Mara: Empty now implements Write too, so Sink isn't really necessary anymore. The 8472: Duplication isn't really a problem. David: Sink name can be clearer. Amanieu: What happens if you Read from Sink? Mara: That isn't implemented. We could also implement it. Then it's identical to Empty. :woman-shrugging: JoshT: We can't merge the types into one type, because code might rely on it being distinct types. David: Don't agree that 'Sink' should be deprecated. The 8472: Don't see any benefit to deprecating it. JoshT: Same. JoshT: We could add a doc comment to Sink saying that if you want Read too you can use Empty. Mara: (joke) Read for Sink should just give you uninit memory. Mara: If we were designing a language from scratch, just having one type for this seems fine. But with what we alread have, I don't see this change improve anything. Amanieu to reply and close. ### (nominated) rust.tf/114564 *Attempt to describe the intent behind the \`From\` trait further* The 8472: It's only guidance Amanieu: In other words, we can always change our minds. The 8472: yeah Mara: We should still make sure we can stand behind this guideline. Amanieu: This mentions IpAddr*, and those are a bit questionable (u32 to Ipv4Addr). David: About the first point: it's not always the case that you can get the original value back out. E.g. ExitCode doesn't have a method for getting the info out. Amanieu: That's fine, it just needs to be lossless. Mara: The doc in the PR says "typically", so might be fine to skip edge cases like those. The 8472: It needs to be injective. David: Yeah but the docs say that you must be able to recover the original value. The 8472: Yeah that needs changing. Josh: Bijective? The 8472: No, the target type can have a larger domain. Conclusion: Seems fine, except the 'recover original value' doesn't always hold. ~~David~~ Josh to reply with examples. The 8472: Perhaps it should say that *if* a reverse conversion exists, then it should be recoverable. JoshT: Scott brought up some examples where some information is lost. Like from Vec to Heap, where the order is lost, but that seems fine. Scott suggested to just use 'should', since it's just guidance. Amanieu: That's fine. ### (waiting on team) rust.tf/114149 *\`read\_dir\` has unexpected behavior for \`""\`* Mara: Read the top comment. Makes sense to me to have "" be the current directory. JoshT: Yes, but also it seems error prone if you didn't get the "" from Path::parent().. Magically having "" mean current directory seems error prone. Mara: The second paragraph explains how we currently have no way to list the current dir without prefixes in the returned paths. That's also an argument for accepting this. Amanieu: Why do the returned paths have a prefix in the first place? Mara: You can choose with .path() or .file_name() if you want the prefix or not. JoshT: The user who says they can't get a non-prefixed path.. does file_name not work for them? Or is it about not having to special case? Mara: If your code can handle both e.g. "data/" and the current directory, you'd use .path(), and it'd be nice to not have "./" on the cwd entries. Amanieu: `Path::parent()` should've returned an enum (current directory vs a directory path). JoshT: Agreed, but it's too late to change that Mara: That still doesn't solve the `./` problem. The 8472: Is the `./` a problem though? JoshT: It is for the user who opened this. Amanieu: Two possibilities: 1) accept "" in read_dir for current directory. 2) we don't change anything. JoshT: On Windows, `read_dir("")` already works. So is that a bug, or is the Unix one a bug? Chris: It happens to work by chance of windows, because we add a `*`. With a different API it wouldn't work. So it's kind of accidental. No other API that accepts path takes empty paths. Mara: How many APIs do we have that accept directory paths rather than file paths? What does `metadata("")` do? Mara: It gives 'not found'. Amanieu: Whatever we decide, we should make the behaviour on Windows and Unix the same. Mara: How long has read_dir("") worked on windows? Chris: Pretty much always. JoshT: I'm feeling sympathetic to the use case of feeding Path::parent() to read_dir(). But it also feels problematic for read_dir("") to work.. I can imagine it'd result in a CVE at some point. Mara: Hard to imagine how that results in a CVE. .join("") also already works. Amanieu: It's not something the os natively does, but something Rust std adds. I don't think we should do that. Mara: But the OS also doesn't have the DirEntry::path() function that retuns the file name with prefix. We have no way to do a read_dir with current dir without prefix on the entries. Amanieu: You can use file_name for those cases. The 8472: Or normalize the paths first. JoshT: Should Path::parent() have returned "." instead of ""? Amanieu: What would parent of "." be then? Amanieu: Relative path's parent() should've returned None. The 8472: You can use parent() to match on, with an arm for "". David: Yes, I use that. Very convenient. Also for making a new PathBuf, to start with an empty PathBuf. Amanieu: Makes sense. Amanieu: canonicalize seems right Mara: No, then you get an absolute path. People want a read_dir without a prefix for the current directory. The 8472: Then strip the prefix? Mara: That might be a different prefix. David: I'm in favor of making read_dir("") work as in this PR. Mara: Me too. David: If you have a Path::new(".") and call .parent(), it returns Some(""). Calling .parent() again returns None. Doesn't seem great that the parent of the current directory (".") is the current directory (""). But perhaps popping components is just what parent() does. David: For `./././.` it pops all of them though. JoshT: Then it might be a bug that the parent of `"."` returns `""` David: Yeah Mara: parent of `"a/."` you get `""`, not `"a"`. So the parent for `"."` being `""` seems like a bug.. Amanieu: Who's in favor of just re-writing the whole path api? (: JoshT: Yes please. David: Doing parent() without file system access is good. JoshT: Absolutely. David: So for the read_dir thing: "" meaning CWD seems fine. Is that a consensus? Mara: I agree The 8472: Might be confusing. JoshT: Agree. It's disappointing that parent() returns "" rather than ".". Mara: The way i see it, you give the 'prefix' to read_dir. It gives the files that start with that prefix. If you give it "", you get the files that have no prefix: the ones in the current directory. David: I agree with that view. Similar as with .join("foo") on an empty PathBuf. JoshT: I wouldn't block this if i was the only voice again, but the docs should be very clear. The 8472: I agree that it isn't great, so you're not the only one. Amanieu: Same. Amanieu: If we change read_dir, should we also make open("") etc. work? Mara: No, because read_dir takes a prefix, not a path. "" is a valid prefix, not a valid path. The 8472: The o.s. takes a path to the dir, not a prefix. Amanieu: Yeah, it's not a tree walk. David: remove_dir_all(""), should that take a 'prefix'? David: Three points of view now: 1. mara: read_dir("") yes, metadata("") no 2. amanieu: read_dir("") no, metadata("") no 3. david: read_dir("") yes, metadata("") yes The 8472: The example in the docs shows "." with "./" in the output. Mara: I still want a way to not get the `./`. The 8472: We can add a prettify method. Mara: No, I *do* want `./` if i give it `.`. I want it prefixed exactly with what i provided. David: Strongly agree. The 8472: It means the same though. David: There is value in showing exactly what the user typed. E.g. 'failed to open ./asdf', if the user typed `.`. JoshT: But tools usually don't allow "" as input. Mara: It does in combination with parent(). E.g. from `"./Cargo.toml"` readdir on parent should give `"./Cargo.lock"`, but from `"Cargo.toml"`, readdir on parent should give `"Cargo.lock"`. Amanieu: Walkdir crate calls fs::metadata on a path as the first thing. So empty string will fail. So metadata should also be changed if we change readdir Mara: It's up to walkdir to decide if they take a path or a prefix.. readdir() uses it as a prefix, because we already join() it into the path(). {no agreement} The 8472: It's a Path problem, not a readdir problem. Mara: Path is a problem, but readdir is a separate problem. I still want readdir("") to work, since it uses that as the prefix it joins into the .path()s. {no consensus} ### (needs decision) rust.tf/93415 *Named format arguments can be used as positional* Is already in FCP to close. ### (new change proposal) rust.tf/libs253 *Provide way to deconstruct std::iter::Rev* ### (new change proposal) rust.tf/libs255 *Adding \`set\_route\` to \`sys::unix::net\`* ### (new change proposal) rust.tf/libs256 *Add a \`CStr::len()\` method* ### (new change proposal) rust.tf/libs257 *Implement \`From\<&'a &'static str\>\` for \`Arguments\<'a\>\`* ### (new change proposal) rust.tf/libs258 *Generify \`BorrowedBuf\` and \`BorrowedCursor\`* ### (stalled change proposal) rust.tf/libs156 *&mut \[MaybeUninit\<T\>\]: Fill with value or with closure.* ### (stalled change proposal) rust.tf/libs155 *Arbitrary alternate flags in \`std::fmt::Formatter\`* ### (stalled change proposal) rust.tf/libs145 *ACP: Additional NonZero conversions* ### (stalled change proposal) rust.tf/libs124 *Integrate \`Error\` trait with panic interfaces* ### (stalled change proposal) rust.tf/libs111 *Restructure ptr\_metadata to minimal support* _Generated by [fully-automatic-rust-libs-team-triage-meeting-agenda-generator](https://github.com/rust-lang/libs-team/tree/main/tools/agenda-generator)_