Try   HackMD

Meeting Link: https://meet.jit.si/rust-libs-meeting-crxoz2at8hiccp7b3ixf89qgxfymlbwr
Attendees:

Agenda

  • Triage
  • Anything else?

Triage

FCPs

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.) - (4 checkboxes left)
  • merge rust.tf/62726 Tracking issue for io_slice_advance - (3 checkboxes left)
  • merge rust.tf/98704 Implement From<OwnedFd/Handle> for ChildStdin/out/err object - (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)

BurntSushi (10), joshtriplett (7), dtolnay (3), m-ou-se (7), Amanieu (5), yaahc (4)

(nominated) rust.tf/92122 Tracking issue for thread local Cell methods

Unresolved question: is it okay/good that .set() skips the initialization? To make this work:

thread_local! { static ID: Cell<i32> = panic!("ID not set on this thread!"); } fn main() { ID.set(123); // OK, no panic. println!("{}", ID.get()); // OK, no panic now that it has been set. }

Mara: If it does exactly T.with(|x| x.set(value)) does, then it first gets initialized and then overwritten. T.set(value) instead skips the default initialization.

David: This seems pretty nice.

Amanieu: Don't like it.

JoshT: Could be a subtle user trap.

Mara: I can't think of any situation where this would cause a bug.

JoshT: Maybe the initializer should be const. {examples of things intializers can do.} Skipping it might result in something go wrong.

David: I can see initializers do that, but not sutuations where you'd also use .set().

JoshT: What's the downside of just always running the initializer, other than losing the panic!() use case?

Mara: The panic use case is exactly the case I care about.

David: Yeah I like that use case. This basically gives us the thread-local version of OnceCell, but without needing new type/api.

The 8472: Would it help to rename it to reinit or something?

Mara: Regardless of whether we have a .init(), I don't think .set() should make it do "init and destroy and set".

Amanieu: Lazy doesn't have .set(). If it had, would it skip initialization?

Mara: OnceCell does have .set() that skips init.

JoshT: In your panic use case, would you call .set() just once or multiple times?

Mara: For the ID use case, just once. (Should probably use a OnceCell.) But for other use cases perhaps multiple times.

JoshT: Should there be a set_once or some other name that implies that you can only call it once and it bypasses initialization?

Mara: That's OnceCell, but now we're talking about a thread local Cell.

JoshT: So what's the use case for setting multiple times?

Mara: Example: output capturing. thread local dyn Write. The default init could open a buffer or file, but if you .set() it before the first print you don't want to directly allocate/open and dealloc/close.

JoshT: That's a valid case.

JoshT: Suppose you do want lazy init and then set?

Mara: Then you just call .with(|x| x.set(..)).

The 8472: Or .replace().

JoshT: Okay, I do think it might be error prone, but I agree that the situation where it matters is very unusual and involves the user doing something exceedingly "clever", in which case they hopefully read the docs. So I'd be fine with it if the docs are clear.

The 8472: They can also wrap the type if they don't want the method.

Amanieu: Agree with Josh. I don't think this will cause issues in practice. If you're exploiting this edge case you're aware of it.

Mara: Docs are clear, including the panic use case: https://doc.rust-lang.org/stable/std/thread/struct.LocalKey.html#method.set

FCP kicked off: https://github.com/rust-lang/rust/issues/92122#issuecomment-1660551128

JoshT: Maybe we should add to the docs that replace is an alternative.

Mara: I can't think of any use case where you'd want that.

JoshT: The docs say "unlike the other methods", so that's good enough.

(nominated) rust.tf/101155 Map EINPROGRESS to io::ErrorKind::WouldBlock

Amanieu: What is EINPROGRESS?

JoshT: EINPROGRESS is not like EAGAIN. It's a failure state, not a 'please wait' state.

{some discussion about when this happens with connect()}

JoshT: In conclusion, you'd handle these differently, so they should be different errors.

Mara: So then the question is if we want ErrorKind::InProgress. I'd think no, because that'd be platform specific.

JoshT: It's pretty standard on unix.

Mara: Nothing would use it on windows right?

JoshT: True, but we have that for more errors. That's fine.

The 8472: Windows would return EWOULDBLOCK in some of those situations. see https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-connect

JoshT: That doesn't seem like the same situation.

Mara: My general worry is that on non-unix platforms it leads people to believe they can handle this error separately, even when it maps to a different (more general) category on e.g. Windows.

Amanieu: Ideally we'd want to map the EAGAIN from windows connect() differently, but I don't think we can do that.

JoshT: That might be an argument for the separate code, then libraries could map them differently.

Mara: Today you need cfg(unix) to handle libc::EINPROGRESS. Adding ErrorKind::InProgress still requires cfg(windows), except you can forget about it and write non-cross-platform code that compiles on all platforms. Bad.

JoshT: We already have many, many cases where different platforms return different errno cases for the "same" error on different platforms.

The 8472: I can't find any other use cases for the error than sockets.

JoshT: There are others too, e.g. filesystem.

Mara: What's the point of having a unix-specific ErrorKind? If you're making unix-specific software, just use libc::EINPROGRESS?

JoshT: EINPROGRESS exists in Windows too. You can still use ErrorKind::InProgress on Windows.

Amanieu: Alternatively, map EINPROGRESS to WouldBlock, and then the user can decide what to do based on the api they called.

JoshT: That seems bad. You check for WouldBlock in an async loop for example.

Mara: Looks like we can't reach a conclusion on InProgress today, but it does sound like we should re-open the ACP.

Josh reopening the ACP: https://github.com/rust-lang/libs-team/issues/92

(nominated) rust.tf/109049 split_array: make methods non-panicking

Notes from before the meeting:
- Jubilee: "I am not really aware of any other code in the standard library that deliberately relies on user-facing "post-monomorphization errors". I believe this may be the first? I believe this should be discussed by the relevant teams before accepting this PR. It's low-risk since this is user-facing, but it is a significant change. The alternative is essentially "wait for GCE to get better before allowing these functions to develop further"."
- Jubilee: Note that while inline_const isn't fully stable yet (and is expected to make this kind of thing more common), it's mostly due to hashing out some compiler details.

Mara: Jubilee notes that the title of the PR is wrong. That is confusing.

The 8472: This panic more eagerly, at compile time.

David: Yeah. We had a similar case where N=0 would not compile. We rejected that on purpose.

The 8472: We don't have the right mechanism for this. You don't want it to trigger in a removed/unevaluated branch.

Thom: We either do it at runtime or compile time, those seem like the only two options.

The 8472: Some checks are delayed until after monomorphization.

David: Link: https://github.com/rust-lang/rust/pull/99471#issuecomment-1555908856

David: Does the same pattern apply here?

Mara: Yes, I think os.

David: I think so too.

David: Someone brought up C++-style constexpr if, but that's probably not available to us.

Thom + JoshT: There are ideas for that, but not coming any time soon.

Thom: It's worth noting that
https://github.com/rust-lang/rust/issues/74985
is listed as blocked on making the N == 0 error fail at compile time. Agree it might not be desirable, but there might be mismatched expectations.

Mara: A tool we have available today is a lint. Like you get for a[100] when it's out of bounds.

Thom: Yeah a lint sounds ideal.

The 8472: Does that work in the generic case?

Thom: That could work.

David: There is one other change in this PR: Some return values become Option.


Out of meeting time.

(waiting on team) rust.tf/114149 `read_dir` has unexpected behavior for `""`

(new change proposal) rust.tf/libs248 ACP: Add `finish_non_exhaustive` to `DebugList`, `DebugMap`, and`DebugTuple`

(new change proposal) rust.tf/libs250 Add LinkedList::{retain,retain_mut}

(new change proposal) rust.tf/libs251 Expose `add`/`sub`/`sub_ptr` on `NonNull`

(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`

  • Note this does include a proposal to expose user-facing API around std::os::unix::net, it's not confined to the inner std::sys module.
  • Jubilee: Seems weakly motivated? If it was an "obvious" cross-platform implementation we could do it anyway, but seems very platform-specific and purely "additive" to std.
  • Jubilee: Note there do seem to be other proposed nightly APIs that are more OS-specific than "any unix" in std::os::unix but I think these should probably be reevaluated if they aren't applicable to all "Unixen". Unices? Units?

(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