Skip to content

Remove AsyncRead::initializer #1761

@Nemo157

Description

@Nemo157
Member

The tracking issue for std::io::Read::initializer makes it sound like that API is definitely going to be changed a lot, it would be good to remove the similar API here until the std API is closer to its final form.

Activity

cramertj

cramertj commented on Jul 23, 2019

@cramertj
Member

Yeah, it seems like there's some push towards having freeze() instead. cc @seanmonstar @RalfJung

RalfJung

RalfJung commented on Jul 24, 2019

@RalfJung
Member

There was a push at rust-lang/rust#58363, but there are two issues with this:

Note however that the current initializer API is incompatible with rust-lang/unsafe-code-guidelines#77, at least if rust-lang/unsafe-code-guidelines#71 ends up deciding that integers must be initialized to be valid.

seanmonstar

seanmonstar commented on Jul 24, 2019

@seanmonstar
Contributor

It seems to me like we're at an awkward point with regards to the desired functionality here.

  • If it's decided that &mut [u8] to uninitialized memory is UB, then we need to add methods like unsafe fn read_uninit(&mut self, buf: &mut MaybeUninit<[u8]>).
  • However, if it is decided to NOT be UB, then we don't need those methods, and don't need to suffer the churn.
Nemo157

Nemo157 commented on Jul 25, 2019

@Nemo157
MemberAuthor

Whether or not an &mut [u8] pointing to uninitialized memory is UB, there was earlier discussion in the tracking issue about how the ceremony in the current API doesn't provide much utility and it could be replaced with a simpler API. It would be disappointing if AsyncRead ended up with a different, more complicated API than Read.

taiki-e

taiki-e commented on Aug 30, 2019

@taiki-e
Member

I think we should choose one of the following:

cramertj

cramertj commented on Aug 30, 2019

@cramertj
Member

+1 for making it unstable. The "simpler API" is unsound, though, so I don't want to do that.

carllerche

carllerche commented on Aug 30, 2019

@carllerche
Member

@cramertj what is unsound about it?

cramertj

cramertj commented on Aug 30, 2019

@cramertj
Member

unsafe fn may_read_buffer(&self) -> bool { true } is incorrect because it is unsafe to implement, not unsafe to call. The only way today to make something unsafe to implement is via a separate unsafe trait, or something which is forced to call an unsafe function, at which point you wind up back at the current initializer API.

carllerche

carllerche commented on Aug 30, 2019

@carllerche
Member

@cramertj makes sense, thanks for the explanation. Is the intent to document the unsafe requirements you describe as part of https://github.com/rust-lang/unsafe-code-guidelines or some other location?

cramertj

cramertj commented on Aug 30, 2019

@cramertj
Member

The requirement that unsafe fn in traits is only unsafe to call, not unsafe to implement? I'm not aware of an existing UCG proposal for that, but I'd certainly be interested in one, yes.

RalfJung

RalfJung commented on Aug 31, 2019

@RalfJung
Member

That seems more like Reference material than UCG. There's no open questions, just things are a bit surprising (rust-lang/rfcs#2585 would improve things IMO by making the role of unsafe fn clearer).

added a commit that references this issue on Oct 1, 2019

2 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @carllerche@seanmonstar@Nemo157@RalfJung@cramertj

      Issue actions

        Remove AsyncRead::initializer · Issue #1761 · rust-lang/futures-rs