text:
Amend RFC 517: Revisions to reader/writer, core::io and std::io

The IO reform RFC is being split into several semi-independent pieces, posted as PRs like this one.
This RFC amendment adds the sections on Reader/Writer revisions, as well as core::io and std::io (which are closely related).
Rendered

I believe I should repeat my question about EINTR here. Is there any agreement on how EINTR is handled by write_all and read_to_end and friends?
The problem is that if EINTR is returned as error by those methods, it means that for example running a strace (along with many other harmless cases for using signals) against process doing write_all would return a NonatomicResult. Which means that to work smoothly write_all must be wrapped in the loop anaway, which defeats the whole idea of write_all (comparing to plain write).
However if EINTR is consumed by internal loop of write_all, sometimes we will get processes which stuck on Ctrl+C forever (or even SIGTERM or any other signal).

@aturon the rendered goes to the old RFC.

@tailhook I think APIs returning NonAtomicResult such as write_all() should consume EINTR inside.  If some people finds it unacceptable, then bare write() is there for them.  As for your concern about those APIs becoming irresponsive, decision on how to handle EINTR doesn't make substantial difference because all the blocking I/O API have the risk of helplessly getting stuck with the so-called "D" state (known as TASK_UNINTERRUPTIBLE for Linux;)  see, for example,

http://stackoverflow.com/questions/1475683/linux-process-states
http://unix.stackexchange.com/questions/16738/when-a-process-will-go-to-d-state

@tailhook

I believe I should repeat my question about EINTR here. Is there any agreement on how EINTR is handled by write_all and read_to_end and friends?

I'd first recommend taking a look at this article about EINTR (just did so myself for a refresher!), it's got some great information about EINTR and what it can be used for. The gist of the message is that EINTR cannot be used to reliably catch a "please interrupt" signal to the program in the case of syscalls like write and read (due to the last race that was mentioned). Another very important note about the article is that EINTR does not exist on Windows.
Due to our commitment to cross platform compatibility as well as the objects being somewhat high-level primitives, I think we will continue to swallow EINTR and the write level for the upper primitives.
As @nodakai mentions, however, it may be useful to handle this case (such as if you're manually using pselect, for example). For these cases I think that we may expose EINTR from the lower-level primitives rather than the higher level primitives.
For example, std::net::TcpSocket would always swallow EINTR, but if you used the AsRawFd trait to get out a &unix::FileDesc (a type not spec'd/planned currently), then write would literally be a call to the write syscall, returning all errors in their full glory.

@nodakai, the issue with "D" state is both rare enough and impossible to do something about so we should not care.
In many other cases Ctrl+C works. Not being able to use is very very annoying. There are precedents of bad SIGINT handling history, for example SVN versions 1.1-1.4 (IIRC) were not handling the signal fast (could linger for minutes) and that was deemed as a bug. And you know, you usually can press Ctrl+/ or just kill process externally, but that does not execute destructors. And I don't wan't another bag of utilities that do same.
@alexcrichton, while I can live with write_all and read_to_end that swallow EINTR (which should be clearly stated in documentation). I believe that not having it in write function is mistake. It means that every small command-line utility that does networking (e.g. a curl-like thing) or reads/writes many files (e.g. like a cp) is not interruptable. And every library that's written in recommended and straightforward way (i.e. not using any kind of asynchronous I/O loop), is not designed to handle interrupts.
Sure, complex programs like network servers which will use low level level API, unblocking calls, asynchronous loop like "mio", and will use signalfd (or polyfill) anyway, so will not suffer this problem.
But then what the high-level cross-platform interface is designed for? Is it for command-line tools that either hang or die without a cleanup? Is this only because they can't do anything better on Windows? Then I guess it's better to turn EINTR to error or even panic on it, in which case if the error is unhandled, at least it will execute all the destructors.
I think we may return some  OperationInterrupted error, but not always when EINTR happens, only when signal handler has specified to do so. The error just expected to propagate up to the main() and exit with some non-zero exit code. The fact that OperationInterrupted is never returned on Windows doesn't make API less cross-platform. It's just random thought, I'm not sure if this is a good idea.

It means that every small command-line utility that does networking (e.g. a curl-like thing) or reads/writes many files (e.g. like a cp) is not interruptable.

As mentioned in the article that I linked to, using EINTR to handle an interrupt for your program in some form is racy (and generally not correct) if all you're using is the write syscall (which is what the write function would correspond to).
I definitely agree that we'll want a nice way to interrupt small programs performing various operations, but I don't think that EINTR is the right way to do so. Doing this, however, would require deeper modifications to the I/O interfaces to enable something along the lines of:

Running arbitrary code when a signal is received. This would likely be on a separate thread which would then signal that an I/O operation elsewhere should be canceled.
Returning control over to the main program. This likely meant that you weren't using write in the first place!

These are both somewhat involved, which is definitely where some high-level abstractions would help out! These may not exist in the standard library immediately, but I'm sure they'll start popping up in Cargo soon though.
To reiterate, handling an interruption via EINTR in write is racy, and as a result it shouldn't be the impetus for returning EINTR from functions at the high level.

I think we may return some OperationInterrupted error, but not always when EINTR happens, only when signal handler has specified to do so.

A crucial part of the stabilization of IoError will be ensuring its future extensibility, so although we may not provide this error quite yet, we could probably do so in the future!

@alexcrichton, I agree with most of what you say, but:

I definitely agree that we'll want a nice way to interrupt small programs.
[ .. snip .. ]
These are both somewhat involved, which is definitely where some
high-level abstractions would help out! These may not exist in the
standard library immediately, but I'm sure they'll start popping up in
Cargo soon though.

Sounds like "never use abstractions in standard library". So why they are there in the first place?
Well, in fact the more I think about it the more I like idea of panic!() on SIGINT/SIGTERM. Of course it should be opt-out (maybe just sigmask is enough). The panic!() may just interrupt program in any place (not only I/O) and execute destructors. Is this at all possible?

I feel like this probably belongs here, but I feel it would be nice to have a "SearchableReader" trait which is like a Reader, but you can change the position inside of it.

Well, in fact the more I think about it the more I like idea of panic!() on SIGINT/SIGTERM. Of course it should be opt-out (maybe just sigmask is enough). The panic!() may just interrupt program in any place (not only I/O) and execute destructors. Is this at all possible?

I don't think so. Unlike a panic! written in by a user, a signal can arrive at any time, and there is no guarantee that at any given moment the stack will be in a valid state to be unwound in the way that panic! typically does.

We'd only be able to panic! on signals inside of functions that are allowed to panic!. We can't have the runtime decide to panic at an arbitrary point.

@tailhook

I definitely agree that we'll want a nice way to interrupt small programs.
[ .. snip .. ]
These are both somewhat involved, which is definitely where some
high-level abstractions would help out! These may not exist in the
standard library immediately, but I'm sure they'll start popping up in
Cargo soon though.

Sounds like "never use abstractions in standard library". So why they are there in the first place?

This is one reason why we're trying to design std::io with future extensibility in mind. While today the primitives may not be so easily interruptible, we want to be able to provide primitives in the future (in the standard library) which do allow this sort of functionality.
I don't mean to say "don't use the standard library" when I say "these may not exist in the standard library immediately", only that advanced functionality is currently not provided on the standard library primitives. We plan to expose the low level details (like handles/file descriptors) to allow the objects to interoperate with other systems and should eventually empower us to write higher level abstractions like something to handle interrupts.
Keep in mind though that this is inherently a tricky problem (especially to do so correctly!). To my knowledge the "accepted practice" for something like this is using select, epoll, or IOCP on windows, all of which are quite complicated systems and will take some time to grow libraries for.
The alternative is providing nothing in the standard library because we can't have complete implementation today, which seems worse than providing a bare-bones primitive?

@Binero

I feel like this probably belongs here; I feel it would be nice to have a "SearchableReader" trait which is like a Reader, but you can change the position inside of it.

Would the Seek trait be what you're looking for?

@alexcrichton: Indeed is, thanks. :)

I did a writeup on an alternative design for the Reader trait: http://discuss.rust-lang.org/t/reader-stablization-errors-and-iterators/1345
Still need to write down what I want to do about Writer  though.

I don't fully understand the discussion about EINTR.  You mentioned a general purpose interrupt interface; how are you going to actually have that interrupt signal the write that it's done without handling EINTR?  Treating that as an implementation detail seems pretty wrong to me.  Like, I realize EINTR isn't great, but I think automatic retrying should at least be opt-out because it certainly isn't always what you want to do.

All: I've pushed a significant update to this RFC, based on discussion here and in the original RFC:
NonatomicResult is dropped. It turned out to be too unwieldy to make use of the intermediate results. In many cases robust usage would require an additional layer of looping and buffering, therefore defeating the purpose of the convenience methods that NonatomicResult was trying to make more robust.
Instead, the philosophy is that read and write provide a tiny veneer over the underlying system calls, passing on all errors and providing a good basis for all blocking IO. Other methods like read_to_end and write_all are just conveniences that are not intended for the most robust applications, so their APIs are made as simple and convenient as possible.
To make this clear, the conveniences are moved off to an extension trait, leaving just read and write/flush methods. Consequently, the traits themselves are renamed to Read and Write, reflecting general trait naming conventions. (See the updated RFC for more details.)
read_at_least is dropped. It wasn't carrying its weight.
All errors are passed through. The read and write methods do not swallow any errors. They aim to represent an idiomatic Rust API that matches the system API as closely as possible.
EOF is now Ok(0). The pros/cons are discussed in the updated RFC text, but this follows discussion on this thread and others.
No zeroing or UB. The text above undefined behavior and zeroing has been removed. This RFC makes a conscious decision to not to address potential security problems with uninitialized data.
@alexcrichton will be updating the section on MemReader and MemWriter shortly.

Great changes.

Sounds good to me!

I'd like to make a further proposal, based on the discussion on the earlier RFC. To allow both (1) re-use of storage and (2) more useful behavior on errors, the various read conveniences would take in a mutable reference to a Vec or String:
fn read_to_end(&mut self, buf: &mut Vec<u8>) -> Result<(), Err>;
fn read_to_string(&mut self, buf: &mut String) -> Result<(), Err>;
fn read_until(&mut self, sep: u8, buf: &mut Vec<u8>) -> Result<(), Err>;

While these APIs are very slightly less convenient, they are potentially much faster, and they follow the same pattern as the read API itself (so there's a consistency argument). We can still provide further conveniences like lines separately.

@nodakai Note that I changed the Seek section per your comment. Please let me know if you have any further comments on this RFC.

There's now a commit which removes MemReader, MemWriter, BufReader, and BufWriter in favor of one Cursor API

All errors are passed through. The read and write methods do not swallow any errors. They aim to represent an idiomatic Rust API that matches the system API as closely as possible.

One consequence of this is that write_all would need a bound on the error type to detect EINTR, which I'm not personally sure is worth it, but certainly a possibility to explore!

After some comments between @sfackler and I, I'd like to take a second to talk about the concrete Err type for the adaptors like Empty, Sink, Repeat, and Cursor<&[u8]> (and friends). Ideally these would all actually be defined to being generic over the error type, but I think that may end up being too painful (requires an E type parameter on each structure). Semantically speaking, none of these structures can return an error, and it would be nice for the type to represent that!
Along these lines, I'd like to propose the following:

A Void type is added to the stdlib. Not entirely sure where, but perhaps std::error for now?
Add impl<E: Error> FromError<Void> for E { ... }
Each structure above has an associated error type of Void.

Using Void should allow us to statically prove that these structures cannot produce an error, but all usage will still be amenable to try! due to the FromError implementation which converts a Void to any other error (because it can't actually ever exist).
Unfortunately, this doesn't work for some other adaptor methods. For example the copy method requires that both R and W have the same associated error type (as well as a number of other adaptors requiring this). This means that the adaptors can't actually be adaptors! To remedy this, I would propose the following method be added to ReadExt and WriteExt
fn map_error<E>(self) -> MapError<E> where E: FromError<Self::Err> { ... }

This adaptor method will allow "subtyping via explicit coercions" in a sense where a Write<Err=Void> could be converted to any other Write<Err=E> (specifically an I/O error!).

Currently Buffer's read_until reads either all characters until and including the delimiter, except if EOF is reached. Also read_line works the same (just with a hardcoded delimiter). The Lines iterator should always return the same "kind" of element. So either the last line always gets a \n added (very ugly imo), or the \n gets always dropped. I also suggest extending the ReadUntil iterator to return an enum with the variants WithDelimiter and WithoutDelimiter so the user can (and has to) distinguish between the situations. The enum would implement two functions, one that always returns the inner value with a delimiter (adding if it's not there) and one that always returns the inner value without a delimiter (truncating if it's there). The reasoning for not doing that with Lines is that you almost never want to know if it was the last line with a delimiter or the last line without a delimiter. There are lots of programs that assume the last character in a file is a newline, having the difference visible in such a high level function will only cause problems.

There are now some more substantial revisions including:

Minor adjustments to the placements of various APIs
Removal of Seek::position
Adding std::void::Void and showing how it is used as an error type.
Explicitly stating tee and broadcast are unstable while their error types are hammered out
A WriteAllError trait was added for the write_all method.
The associated error type for each adaptor has been spelled out.

I'm particularly keen on feedback for WriteAllError as I don't think that its complexity justifies its existence currently, but I'm curious what others think about it. One alternative would be to move write_all and write_fmt to a separate extension trait (not in core) which only works with writes of one kind of error (io::Error).

@alexcrichton great work! Any reason WriteAllError isn't used for read_to_vec and read_to_string too?

After going about and implementing these changes, I've learned a few new pieces of information which I think may influence the outcome of this RFC. Primarily, I'd like to take some time to talk about the associated Err type of each trait. Working with an associated error type turned out to be surprisingly difficult and I'd like to take a moment to review the pros and cons of using an associated error type. First, some pros:

Using an associated type allows a good bit of the std::io infrastructure to reside in core::io, especially the adapters, implementations on memory buffers, etc.
These adapters and core implementations can all have an error type of Void which is a static guarantee that the object will never produce an error. This is not only good for documentation purposes but it can also allow for interesting compiler optimizations down the road.
It is much more flexible to operate generically over an error type. Each I/O reader and writer can have their own custom error tailored to their own implementation. This also means that a good deal of information about each error is statically known at compile time (variants are known and can be matched against for example).

From my implementation, however, I have discovered a number of cons to this strategy:

The write_all method will require a bound of one form or another. The write_all function returns () and must be able to handle the case that Ok(0) is returned from the underlying call to write. Currently this is manifested as a WriteAllError bound, but another option is bounding the error by FromError<EndOfFile> where EndOfFile is a unit struct in std::io. Both of these do get somewhat cumbersome, however, as it means that this bound must be propagated outwards to anyone calling write_all. Even worse, it means that write_all cannot be called on any of the core adapters as the error type Void does not implement FromError<EndOfFile>.
Different writers can obviously have different forms of errors, which necessitates the existence of the map_err adaptor which will map a writer from one error type to another. This adaptor may end up being very frequently used to interoperate two I/O streams as it's highly likely that their error types are separate.
Each of the Read, Write, and Seek traits have a separate associated type as an error. This means that a bound today of T: Writer + Seek must become T: Write where T: Seek<Err=T::Err> which is somewhat more cumbersome to write.

In isolation all of the cons can likely be addressed via various tweaks to this RFC, but overally I've come to the conclusion that an associated type may not be the best choice for ergonomics. I found myself bending over backwards definining a great multitude of error types only to realize at the very end that FromError<EndOfFile> wasn't implemented somewhere along the way. I was experimenting moving over rbml and our metadata encoding to core::io and was unable to do so as the bounds ended up getting in the way.
So given all this, I would recommend that the RFC moves back to where we are today with an io::Error type which is the returned value from the Read and Write traits. I believe it's still possible to largely get some of the pros of the associated error type:

Tweaks io::Error could erase the internally stored String allowing much infrastructure to reside in core::io if necessary.
This pro will likely not be able to be recovered without an associated type.
Heavier usage of Box<Error> with the addition of downcasting would likely be able to recover most of this aspect. Much of this depends on the exact design of io::Error itself, however.

I also think that there are a lot of design questions around io::Error itself (regardless of whether an associated type is used or not), but I think that many of those are best left to an RFC. For now the Error type should likely be as conservative as possible in order to allow for future growth.
I'm curious about others' lines of thought as well on this though.

@Ericson2314 I think that my thoughts above may answer your question in an unexpected way :)

@alexcrichton You seem to be advocating for ergonomics > expressiveness in that all the upsides are expressiveness, and all the downsides are ergonomic. This strikes me as a scary route to take for such an important trait.
I am messing around writing a kernel with an esoteric IO design, and the ability to have custom errors and these traits in core is of utmost importance to me usecase (so yes I'm a bit biased). More broadly, we loose all the benefits of Void which paved the way to make things like parsing buffers much more pleasant.
If we could write a subtrait that fixed an associated type in the super trait (this ICEd last time I tried it), we could make io::Error-Fixed convenience traits in std, which would alleviate any ergonomic problems for the common case, while not negatively impacting those who need the extra power.

Also, new insight: For the atomic calls write, no EOF error case is necessary, and indeed it is more correct not to have one in order to avoid any redundancy. In the looping case however, the EOF error is necessary. I therefore advocate:
fn write_all<Grr>(&mut self, buf: &[u8]) -> Result<(), Grr>
    where Grr: EofAssociatedConst,
          Grr: FromError<Err>,
          Self::Err: PredTransient;

Yes, it is complicated, but this actually has the desired semantics: write_all into a Vec can now return e.g. Result<(), EofOnly> or something. PredTransient can also be used for the looping reads.

@Ericson2314

You seem to be advocating for ergonomics > expressiveness in that all the upsides are expressiveness, and all the downsides are ergonomic

In some sense this may another way to say what I concluded, but the reality was that I could barely get everything to work with a generic error type. I never actually got the compiler compiling (it's just using a Write instance into memory!) and I suspected that deeper changes would be required, such as manifesting the type:
MapError<ByRef<'a, Cursor<Vec<u8>>>, RustcCustomErrorForEof>

I also don't necessarily think we're losing too much expressiveness with a concrete error type. The only piece I believe we're losing is the ability to say that you're returning the "error" Void as opposed to any error at all, which in general seems like a minor optimization for the usability wins. I do not have data either way on this, so I'm mostly just talking from my gut unfortunately.

If we could write a subtrait that fixed an associated type in the super trait (this ICEd last time I tried it), we could make io::Error-Fixed convenience traits in std, which would alleviate any ergonomic problems for the common case, while not negatively impacting those who need the extra power.

Yes this was suggested my by @aturon, and I didn't actually try pursuing the route once I started tallying up all the grievances of using an associated error type. I do believe that each problem in isolation can be solved (such as this solving T: Write + Seek) but overall I found it just too painful to work with. Although I would not say that I didn't miss something either!

Yes, it is complicated, but this actually has the desired semantics

This is largely along the lines of the currently proposed WriteAllError trait, but the really unfortunate part is that this propagates almost everywhere. The ugliness is not contained to just the function definition as whenever you call .write() you probably really want write_all. (or at least so I found in my initial testing).

Whats the reasoning behind the ByRef type? Seems to me that these two should be equivalent anyway so why complicate things with the ByRef type?
impl <'a, T: Trait> Trait for &'a T { .. }
impl <'a, T: Trait> Trait for ByRef<'a, T> { .. }

I was noticing that the current Reader doesn't have a way to peek to see if there is data waiting to be received... There is try_recv but it will receive the data!

@alexcrichton

This is largely along the lines of the currently proposed WriteAllError trait
First of all, my write_all was supposed to be based on that WriteAllError. I don't mean to claim I independently invented the things there, just tweaked it a bit.

Anyways, thinking back to my experience working with generic errors, the basic problem is Rust doesn't let you do where E: Foo<Bar> + Foo<Baz>. With this, rather than having to eagerly map_error all over the place, you just add more FromError constraints. For example, my write_all would become:
fn write_all<Grr>(&mut self, buf: &[u8]) -> Result<(), Grr>
  where Grr: FromError<EndOfFile>,
        Grr: FromError<self::Err>,
        self::Err: PredTransient;

This has the added bonus of fulfilling a similar role to something like @reem's https://github.com/rust-lang/rfcs/pull/402. Maybe the application is instantiated with a big catchall error type like IoError, for a less efficient run-time, but at compile-time the FromError constraints precisely track what sorts of errors are possible in generic code. In this sense I view the propagation of constraints as an asset, especially since their regularity (mostly FromError) makes things easier to grok.

I also don't necessarily think we're losing too much expressiveness with a concrete error type. The only piece I believe we're losing is the ability to say that you're returning the "error" Void as opposed to any error at all, which in general seems like a minor optimization for the usability wins. I do not have data either way on this, so I'm mostly just talking from my gut unfortunately.

As hinted above, I prefer the error type to as precisely as possible track what could go wrong. Any hardcoded catchall error type makes this impossible. Granted, for anything wrapping syscalls the OS semantics already preclude this, but it is nice to leave the opportunity open for Rust-land things.

I didn't see anything about pread/pwrite.  That is probably more efficient than seek, and people doing random IO might do it, and a quick search shows windows supports it too.   (I'm not suggesting it's high priority though, certainly compared to error handling, signals, FD safety)

@tailhook: You wrote "It means that every small command-line utility that does networking (e.g. a curl-like thing) or reads/writes many files (e.g. like a cp) is not interruptable."
I think you might be misunderstanding what caught the subversion hang (which I have experienced too).  I'm pretty sure subversion was blocking SIGINT altogether, instead of leaving at its default disposition which terminates the process.  So regardless of whether rust retries EINTR for normal writes or not, your trivial file utilities will most certainly be interruptible (killable) by ctrl-c and sigterm.
I think the 'handle ctrl c but due something special', which for example "yum" does to pick an alternate mirror, is somewhat niche and could be handled using non blocking io in a select loop that also reads a signalfd or simple byte pipe.   I do think you have to pick who you optimize for and I think retrying EINTR everywhere could help the 'little guy' who just wants something to work 100% of the time and not fail randomly (but its still killable via ctrl-c as I mentioned earlier).  People who want magic ctrl-c handling can put in a little more work and go non-blocking.
Also see this python pep: https://www.python.org/dev/peps/pep-0475/

I think you might be misunderstanding what caught the subversion hang (which I have experienced too). I'm pretty sure subversion was blocking SIGINT altogether, instead of leaving at its default disposition which terminates the process. So regardless of whether rust retries EINTR for normal writes or not, your trivial file utilities will most certainly be interruptible (killable) by ctrl-c and sigterm.

No I'm not misunderstanding. The issue is that as long as you need some trivial thing like clean temporary files on exit you can't leave SIGINT default. (And that was the case for subversion, if you just kill it you needed to run "svn clean" or something to fix broken working copy). The problem with utilities that doesn't cleanup for themselves is even bigger than ones with hanging on SIGINT. Because if I really want to stop something without cleanup I can always kill it with KILL (9) signal.

golang loops on EINTR during file write: https://golang.org/src/os/file_unix.go line 218 search for eintr.  It also appears they prefer SA_RESTART behavior.  https://groups.google.com/forum/#!topic/golang-dev/ijxJiyyr0P4
BSD also restarts system calls by default (SA_RESTART behavior), unfortunately posix did not and that's why we're having this discussion.  But rust is cross platform and should abstract this; if your program runs fine on BSD it shouldn't fail randomly on linux due to some lib you don't know about having a SIGCHLD handler.
@tailhook programs should be crash safe and not require stupid crap like svn clean if they were killed.  Use something like sqlite that has a journal.  regardless, you can call unlink() in a signal handler safely if you really want to 'delete a temp file', then exit.
Simple code should stay simple, e.g. users should not have to care about retry loops, nobody is going to get that right, and the symptoms are going to be hard to reproduce, and even potentially platform (bsd/linux) dependent.

@tailhook "The problem with utilities that doesn't cleanup for themselves is even bigger than ones with hanging on SIGINT. "
Name some.  I never assume ctrl-c will do a 'graceful' shutdown and am usually annoyed when programs try to intercept it.  ctrl-d, on the other hand, is graceful logout / eof.   Regardless, I'm not suggesting rust code can't get alerted to signals.  It just needs to use modern mechanisms that don't inflict grief on 80-99% of other devs.

@tailhook "Well, in fact the more I think about it the more I like idea of panic!() on SIGINT/SIGTERM. Of course it should be opt-out (maybe just sigmask is enough). The panic!() may just interrupt program in any place (not only I/O) and execute destructors. Is this at all possible?"
As @alexcrichton mentioned, that's racy and would require rust installing its own signal handler for SIGINT which would conflict with easy use in FFI situations.  Python can do that, but that's different, and I would guess it can only do this in 'all situations' because it's interpreted.  And speaking from experience, having every possible line of python code being able to throw a KeyboardInterrupt - even in finally handlers - is insane.  It's literally impossible to test all the code paths.    See https://www.python.org/dev/peps/pep-0475/   "Use a I/O multiplexing function like select() with the Python signal "wakeup" file descriptor: see the function signal.set_wakeup_fd() ."  that's saner to test.

@Ericson2314

As hinted above, I prefer the error type to as precisely as possible track what could go wrong. Any hardcoded catchall error type makes this impossible.

Ah I may have forgotten to mention this, but I've been operating under the assumption that a Box<Any> would somehow get able to be placed in a io::Error so you can put truly custom data and downcast it out. This means that you still don't have as many static guarantees, but it's at least not entirely impossible.

@kjpgit

I didn't see anything about pread/pwrite.

Indeed! I suspect that this would be a separate trait, though (maybe ReadAt?) which would be a backwards-compatible addition.
In terms of EINTR, I'm a bit torn here. On one hand I've never wanted to receive EINTR and I'm glad when anything handles it, but on the other hand I can't say that it's useless to receive EINTR. Additionally, with SA_RESTART it does mean that applications can totally ignore EINTR altogether. In theory if a library it's using registers a handler without SA_RESTART then the application could re-register the handler with SA_RESTART and delegate to the other library (in theory).
There definitely seems to be a lot of precedent for handling EINTR in a library, but the standard library has also been moving in a direction of exposing more of the underlying system. There are certainly utilities to handle EINTR seamlessly, but when they're not used it can certainly be quite annoying to have to deal with them. I think I'll have to keep mulling this one over.

@Marwes

Whats the reasoning behind the ByRef type?

There is currently good precedent with iterators that this is a useful type to have. There will be an implementation of Writer for &mut T where T: Writer still (in addition to being on ByRef). To me it is more pleasant to say foo.by_ref().baz() instead of (&mut foo).baz() (for various adapters). It is true that ByRef is not strictly necessary, but I personally think that it improves ergonomics.

@Hoverbear

I was noticing that the current Reader doesn't have a way to peek to see if there is data waiting to be received... There is try_recv but it will receive the data!

Are you thinking of Receiver when you mention try_recv? I'm at least personally unaware of peeking at data in a file descriptor, for example.

@alexcrichton I'm afraid its static guarantees, and not run-time specificity, that I care about, so injecting a Box<Any> in an IoError really doesn't help. Does it make sense to you that having multiple FromError bounds would help? Do you have a commit/branch of trying to implement it the original ways before you gave up on associated types that I could look at?

@Ericson2314

Does it make sense to you that having multiple FromError bounds would help?

I suppose not entirely. I found it painful to have one bound on the error of write_all when porting rbml, but I could be misunderstanding something!

Do you have a commit/branch of trying to implement it the original ways before you gave up on associated types that I could look at?

I do indeed! https://github.com/alexcrichton/rust/tree/core-io
It looks like I never pushed the commit for converting librbml to std::io (from std::old_io), and unfortunately I think that part is lost now :(.

@alexcrichton Thanks! Time permitting I will edit it to show what I envision the types should look like if rust supported bounds like that.

@alexcrichton @kjpgit If we add pread(2), then we would also want equivalents of POSIX readv(2) and de facto standard preadv(2)
(But note that the Win32 equivalent of the latter ReadFileScatter() is an async API.)

@alexcrichton This is what I have so far: https://github.com/RustOS-Fork-Holding-Ground/rust/tree/core-io. Besides my aforementioned changes to write_all, basically what I've done is quite analogous to implicit widening for number types. For the user, the difference is whereas they might need to map_err before, now they could end up with under-constrained expressions and need to explicitly instantiate things. This, for example, occurs with try!(copy(read, writer)).
I do think this counts as an improvement however -- rustc can easily give suggestions for possible types in an under-constrained situation, especially for situations like this where the ambiguous types seem to be either independent, or dependent but "adjacent" (e.g. chain(r1, chain(r2, r3)). Also, under-constrained types just seems less scary than over-constrained types.
[Also I think I might have just thought of a partial solution to insufficient-constraint problem that works for both this and implicit widening without (much) garbling of the inference algorithm. If (a) type inference uses default type arguments to resolve ambiguities, and (b) optional bounds can be used for default arguments, a LeastUpperBound trait can be defined, where the least upper bound is an associated type. For any two types where the trait is implemented, it will give yield a single default argument.]

On Tue, Jan 27, 2015 at 11:58 AM, Alex Crichton notifications@github.com
wrote:

@Ericson2314 https://github.com/Ericson2314
As hinted above, I prefer the error type to as precisely as possible track
what could go wrong. Any hardcoded catchall error type makes this
impossible.
Ah I may have forgotten to mention this, but I've been operating under the
assumption that a Box would somehow get able to be placed in a
io::Error so you can put truly custom data and downcast it out. This
means that you still don't have as many static guarantees, but it's at
least not entirely impossible.
@kjpgit https://github.com/kjpgit
I didn't see anything about pread/pwrite.
Indeed! I suspect that this would be a separate trait, though (maybe
ReadAt?) which would be a backwards-compatible addition.
In terms of EINTR, I'm a bit torn here. On one hand I've never wanted to
receive EINTR and I'm glad when anything handles it, but on the other
hand I can't say that it's useless to receive EINTR. Additionally, with
SA_RESTART it does mean that applications can totally ignore EINTR
altogether. In theory if a library it's using registers a handler without
SA_RESTART then the application could re-register the handler with
SA_RESTART and delegate to the other library (in theory).

Agreed we should think about the FFI / legacy code case.  Rust could use a
C library or rust could be part of an existing large c++ or python program
(for gradual migration), and might not be able to change SA_RESTART
semantics of the parent application.  It would be a bummer if simple rust
code (read a config file using std::io) breaks randomly due to EINTR
breaking the FFI API boundary.
Note that there are still things that SA_RESTART doesn't change, like eintr
on epoll/poll/select/nanosleep/usleep, and sockets where you previously set
a timeout on using setsockopt().  (linux man 7 signal)
I think a simple cross platform io library should try to shield people from
this cognitive load; people who really understand EINTR and actually want
it should opt in.  Do you really want to debug 'oh, you compiled that c
library with gcc but _BSD_SOURCE wasn't set, so you didn't get SA_RESTART
semantics via your signal() setup, so your rust code then got EINTR due to
SIGCHLD signal, even though on mac it worked fine...
Unfortunately either way it's tough: either rust punts to the user and they
could have a hard time, or rust handles eintr loops internally but that
rust code (those loops) are also really hard to test, and could have bugs
(updating byte counts or time).

There definitely seems to be a lot of precedent for handling EINTR in a
library, but the standard library has also been moving in a direction of
exposing more of the underlying system. There are certainly utilities to
handle EINTR seamlessly, but when they're not used it can certainly be
quite annoying to have to deal with them. I think I'll have to keep mulling
this one over.
—
Reply to this email directly or view it on GitHub
https://github.com/rust-lang/rfcs/pull/576#issuecomment-71694941.

@nodakai

@alexcrichton @kjpgit If we add pread(2), then we would also want equivalents of POSIX readv(2) and de facto standard preadv(2)
(But note that the Win32 equivalent of the latter ReadFileScatter() is an async API.)

Good point! I think these would definitely work well as separate traits (or perhaps even inherent methods to start out with). With the I/O prelude it should be relatively easy to add new traits in the future as implementations and designs grow.

@Ericson2314

@alexcrichton This is what I have so far: https://github.com/RustOS-Fork-Holding-Ground/rust/tree/core-io.

Thanks for investigating! I had thought at one point that the adaptors could do the auto-mapping for your (e.g. each one uses FromError to transform errors as I believe you've done), but I shied away from that design as I felt it was conflating synonymous adapters. For example foo.chain(bar) in yours I believe is actually the equivalent of: foo.map_err().chain(bar.map_err()).map_err() (although I think you called it upcast_err). I was worried about combining that all into one.
I also found that writing the generic code was somewhat pleasant, but the real pain hit once you started to use it all. Did you get a chance to try porting librbml over to using it?

@kjpgit

Unfortunately either way it's tough: either rust punts to the user and they
could have a hard time, or rust handles eintr loops internally but that
rust code (those loops) are also really hard to test, and could have bugs
(updating byte counts or time).

Yeah this is basically how I feel as well. I haven't been able to come up with a silver bullet yet which would give the ability to handle EINTR if you really want to and also ignore it if you don't necessarily want to deal with it.

I'd like to propose additional traits to read and write chars, not bytes. E. g. StringWrite and StringRead.
Java has separate bytes and string stream class hierarchies: InputStream/Reader and OutputStream/Writer. C# also has similar classes.
Such types would be useful in Rust.
trait StringWrite {
    // not sure about names and signatures
    fn write_str(&mut self, ...) -> ...;
    fn write_char(&mut self, c: char) -> ...;
    ...
}

trait StringRead {
    fn read_str(&mut self, ...) -> ...;
    fn read_char(&mut self) -> ...;
    ...
}

StringRead should be implemented for any Read (possibly as super trait), and similary StringWrite should be implemented for any Write.
These traits are important for two reasons: better code readability and flexibility.
Readability: Lots of libraries only operate chars, not bytes. For example, JSON encoder/decoders, HTML writers, program source parsers and so on. These libraries accept Reader/Writer traits as parameters now, whereas they can use much narrower interface StringRead/StringWrite. Narrower interface is better for understanding what code does (works with texts, not binary data).
Flexibility: Not every written string ends up in bytes array. Sometimes, strings are

encoded using different encodings than UTF-8
just collected in memory to be processed as sequence of Unicode chars

Having separate traits StringRead and StringWrite makes it possible to have alternative (to Read and Write) implementations that do not need to care about byte-related functions.

@stepancheg, I'd like to support this.
Java's Reader and Writer, while indeed being character-based streams, are heavily dependent on transparent encoding/decoding support: you can wrap any InputStream/OutputStream with InputStreamReader/OutputStreamWriter, providing an encoding, and the library will automatically decode/encode characters from/to bytes. Inability to do this is a serious drawback, IMO.
The question remains if these traits do belong to the standard library, though. Without proper encoding support they are not that useful.

@netvl If we allow encodings to be selected at runtime, that means adding conversion tables for every single possible encoding, which is a bad idea for binary size.

I would like to talk about Unicode string I/O.
In the former design, print! always writes the raw UTF-8 stream to the screen. Which is fine in Linux when the terminal is correctly set. However it is a pain for Windows (the output is totally unreadable especially for Asian users).
As I know, Windows has provided {Read,Write}ConsoleW for UCS-2 based console I/O, which suits our need.
Solution:
I hope to add a Raw/Cooked switch. When stdio is a terminal (isatty in POSIX or GetConsoleMode in Windows), Coooked is on default. When stdio is a pipe, Raw is on default.
In Raw mode, strings are transferred in UTF-8, and binary data are transferred as is. The Windows implementation uses {Read,Write}File API to do I/O.
In Cooked mode, strings are converted to "local charset" -- LC_MESSAGES for POSIX and UTF-16 for Windows. The Windows implementation uses {Read,Write}ConsoleW to do I/O. It is okay to do the conversion lossy, because the strings are sent to human eyes, not used by any other applications.
Raw/Cooked mode switch should exist in all three std{in,out,err} and can be changed with code.

@m13253 Rust already uses ReadConsoleW and WriteConsoleW on Windows. What you describe already happens automatically internally in libstd. Depending upon whether GetConsoleMode succeeds we either use the console methods, doing unicode conversions, or we just ReadFile and WriteFile to the pipe.

@netvl

The question remains if these traits do belong to the standard library, though. Without proper encoding support they are not that useful.

Consider a situation like this: there's a library that (for example) parses/serializes CSV, and another library that implements some unusual encoding like windows-1251. These libraries need common interface, which is a stream of chars.
These char-stream interfaces could be a part of another library, and both CSV and windows-1251 libraries could have a dependency on that third library. But interface is so common and small, so it it is better to include it into stdlib.
In another words, char-stream interface is not that useful per se, but it is a glue to connect independent libraries.

@alexcrichton Sorry, I finished that those commits quite late in the morning so I didn't get to working on librbml. Seems like it might be easiest to rebase off your later version to get std::old_ioetc. I'll try that.

for BufferedWriter, it might be nice to enforce the buffer is a minimum size - at least the largest primitive type you expect to write into it - to go from 2 branches to 1 in the write_u8,i32,etc.  I noticed you can actually set a zero byte buffer today and it won't crash, which is cute, but not quite the purpose I imagine a buffer to be.

@alexcrichton has been doing a nice job shepherding this along while I've been away from the thread, but I wanted to give some further replies here. To keep the noise down, I'm going to do a bulk reply in this comment.
====
@oli-obk

Currently Buffer's read_until reads either all characters until and including the delimiter, except if EOF is reached. Also read_line works the same (just with a hardcoded delimiter). The Lines iterator should always return the same "kind" of element. So either the last line always gets a \n added (very ugly imo), or the \n gets always dropped.

This is unfortunate! (I'm somewhat surprised by the current implementation). I was wondering, though, if it's feasible to simply "split" on the delimiter (in the sense of the split method), so that the delimiter is never included and if there is a final delimiter prior to EOF then the last line is empty. That seems like the behavior I'd expect, but perhaps there are downsides.
====
@Marwed

Whats the reasoning behind the ByRef type? Seems to me that these two should be equivalent anyway so why complicate things with the ByRef type?

Good point! I was just following existing precedent here, but with the various changes that have taken place in the trait structures, this does seem viable, and much nicer.
Interestingly, this means that x.by_ref() is basically (&mut x). (pointed out by @huonw)
====
@stepancheg, @netvi, @m13253
Text-oriented streams are a good idea and something we’d like to pursue eventually. We’ve already postponed an RFC on the topic.
That said, I believe that such functionality can and should be added independently of the conservative byte-oriented reader/writer approach here, and should probably start out in the crates.io ecosystem.
We need to move forward on these RFCs soon, so I'd like to keep discussion focused on cleaning up what we already have, leaving aside future features that are backwards-compatible to add.

Dropping the '\n' from read_line means that read_line + write is no longer lossless.

@mahkoh

Dropping the '\n' from read_line means that read_line + write is no longer lossless.

That's true, and unfortunately read_line + writeln! doesn't quite work either since you can end up with an extra newline. But this is likewise a concern for split on strings; do you have an argument for why they should be inconsistent, or do you think split should also behave this way?

Spitting strings at a char returns a lossless iterator.

@mahkoh

Spitting strings at a char returns a lossless iterator.

Would you mind being a bit more expansive here? I must be missing something. I was claiming above that read_until could be made to work like split, and therefore be lossless. Do you disagree? I thought the point you were making was just that actually using that losslessness was a bit painful, in that you can't simply write the results, but it seems like you're arguing something else, but this terse comment isn't enough to tell.
In any case, it's been pointed out that keeping the separator has advantages with respect to cross-platform concerns, since there's \r\n to contend with on Windows (what exactly would be stripped, then?). That alone is a fairly strong argument in favor of keeping them.
(There's also the fact that keeping the separators has a fair amount of precedent in existing libraries.)

You said above

That's true [...] But this is likewise a concern for split

Which implies that lossiness is a concern for split which it is not. If you use split with a char then you can always reproduce the original string by inserting that char between two elements.

I thought the point you were making was just that actually using that losslessness was a bit painful

I actually just mentioned that removing the \n turns it from lessless into lossy. It's not painful to use because you can use a trim function that removes the \n.

as long as both Lines iterators from io and str are behaving the same. This should be noted as a general requirement for all splitting iterators or functions.
My preference stays with not having the separators in the parts and having an empty last part if the sequence ends with a separator.
as i noted in an inline-comment, io::Lines and str::Lines behave differently:
fn main() {
    let x = "hi\nbye\ncake\n";
    let mut y = x.as_bytes();
    for l in y.lines() {
        println!("{:?}", l.unwrap());
    }
    for l in x.lines() {
        println!("{:?}", l);
    }
}

Output:
"hi\n"
"bye\n"
"cake\n"
"hi"
"bye"
"cake"

I'd prefer read_line to stay the way it is. The lines iterator is not that suitable for handling input from stdin.

I want to try to summarize the current state here, to help move things forward.
It seems like there's reasonable consensus around most of the basic structure here, in terms of slimming down the traits, sticking closer to single syscall semantics, EOF handling, and so on. I think the few remaning items should be quickly resolved or pulled out to separate discussions so we can make progress.
The items still being debated are:
EINTR handling
It's not completely clear when this should just cause a retry. However, this RFC is mainly covering the core traits for reading and writing, and not specific APIs. I think our best way forward here is to have the core read/write operations map as directly to the underlying system APIs as possible, producing an error on EINTR, so that they can serve as a lossless building block for other abstractions.
Conveniences like write_all or read_to_end, on the other hand, should retry on EINTR. This is partly under the assumption that in practice, EINTR will most often arise when interfacing with other code that changes a signal handler. Due to the global nature of these interactions, such a change can suddenly cause your own code to get an error irrelevant to it, and the code should probably just retry in those cases. In the case where you are using EINTR explicitly, read and write will be available to handle it.
read_until's behavior
Does it include the delimiter in the strings it produces (as today's API does)? I think the right approach is for read_line/read_until to retain the delimiter, while lines consumes it. This keeps consistency with the split iterator, and also existing precedent (and useful coding patterns) for read_line; it seems like the pragmatic solution.
close
The discussion about close is probably more relevant on RFCs for specific IO APIs.
That said, I believe that close is backwards compatible to add later, and am personally not yet convinced that it's needed given the existence of flush. There's now a separate RFC on this topic, and I think the discussion should move there (but shouldn't block this orthogonal RFC).
stdin/stdout/stderr:
I think this part of the RFC is underdeveloped, and I'd like to have a separate discussion about it. I propose to remove it from this RFC for now.
Taking &mut Vec<u8>
I made a suggestion about an API change that hasn't yet been incorporated into the RFC. I'll plan to update the RFC with it, but please give feedback on this idea if you have any.
Various names:
There are a few open bikesheds here that will need to be decided, but nothing too major I think.

Oops, my writeup above left out the associated error type change, which is still semi-open. However, given the limited time and serious ergonomic pain, I think we should probably be conservative here and stick with the known-to-work style of today's API.
@Ericson2314 I believe there may be a path toward generalization in the future by using defaulted associated types and type parameters. I'd be glad to try to hash this out with you; I feel like especially if we can future-proof against something more expressive, there's not much reason to take a risky gambit now.
(And while I agree that this is ergonomics vs expressiveness, it's crucial to consider the details on each side. The expressiveness gains here are, in my opinion, not very important in practice except in some marginal cases, but would impose a hefty ergonomic tax on everyone using IO.)

I've updated the RFC with most of the changes discussed above. No change has yet been made to names around Seek.

I feel pretty confident about the changes as outlined by @aturon.

I have an implementation of this RFC here now: https://github.com/rust-lang/rust/pull/21835
I will update the PR with any future tweaks to this RFC, so it's by no means set in stone!

The removal of the associated error type is a major step backwards. One of the nicest parts of this RFC was making I/O more portable, flexible and possibly moving it into core. Without the associated error type, the only errors that can be returned are again the errors "sanctioned" by the module; OS errors and various errors required by the adaptors. @alexcrichton's PR implements a "Custom" error type, which being in a Box is simply insult to injury as it forces I/O out of core completely.
I think attempting to solve the mentioned problems in a way other than simply removing the associated error type is worth it, rather than doing a massive reform only to return a major part to the sad state it already was in.

You might want to change the Seek::Start argument from u64 to i64 for consistency.

Some concerns from @mahkoh on the implementation:

https://github.com/rust-lang/rust/pull/21835#discussion_r23897631
The flush method does not indicate how many bytes were actually flushed, which raises a question about retries and atomicity with BufWriter, for example. The internal buffer may not be entirely written on a flush(), and it hasn't been spelled out, but what should happen? I would think we could do:

Return an error if the single call to write did not write the entire buffer.
Use .write_all to write the entire buffer. Note that this will ignore Interrupted and friends.

Just a point to clarify.

https://github.com/rust-lang/rust/pull/21835#discussion_r23898163
Should flush return Ok by default? We may want to make an explicit decision about the default here (there hasn't been much discussion on the flushing topic)

https://github.com/rust-lang/rust/pull/21835#discussion_r23898182
A suggestion that write_all should return Result<u64> instead of Result<()>

https://github.com/rust-lang/rust/pull/21835#discussion_r23898240
A suggestion to use i64 in SeekFrom::Start instead of u64. Note that this would also raise a question as to why it takes a payload i64 instead of a second argument.

@mahkoh if I misrepresented anything, please let me know! I'll try to copy over future worries to this RFC as well.

It seems the issues with the associated error types mentioned by @alexcrichton are quite easily solved and don't load to any major ergonomic issues.
(I went the wrong way about doing this initially, but it really isn't that bad)

A FromError<EndOfFile> bound on the Err type of write_all is definitely the way to go, FromError<Void> can be blanket impl'd over all types as Void is uninstantiable, which sorts out the Void related problems.
For the latter two points: Write, Read and Seek returning different error types is a good thing for both flexibility and sanity; and so is awareness of that. Writing code that can deal with this does involve some usage of map_err, but it doesn't lead to any major ergonomic issues.

See this branch: https://github.com/Tobba/rust/tree/core-io which ports librbml to an (incomplete) implementation of a proposal for the I/O design.

@Tobba

See this branch: https://github.com/Tobba/rust/tree/core-io which ports librbml to an (incomplete) implementation of a proposal for the I/O design.

One of the key points I personally found was that porting librustc to use librbml was incredibly painful, I never actually got it to compile!
The rbml implementation does not necessarily leverage try! to its fullest extent. I noticed that everything which used to be try!(foo) is now try!(foo.map_err(|err| SomeError(err))) which definitely seems like an ergonomic regression to me personally.
I would encourage you to write some tests for the various primitives as well because it ended up leading to tests looking like this versus looking like this

@alexcrichton

Regarding the map_err, if one does not wish to disambiguate errors, they can enforce that the various errors are the same, or FromError to a common "super type". Then that won't be necessary.

In libcoretest, making the method call  .tee<EndOfFile> should be all that is change wrt the clean version.

I would demonstrate these things myself, but I cannot build the standard library ATM.

Some replies to @mahkoh's concerns:

https://github.com/rust-lang/rust/pull/21835#discussion_r23897631

The flush method does not indicate how many bytes were actually flushed, which raises a question about retries and atomicity with BufWriter, for example. The internal buffer may not be entirely written on a flush(), and it hasn't been spelled out, but what should happen? I would think we could do:

Return an error if the single call to write did not write the entire buffer.
Use .write_all to write the entire buffer. Note that this will ignore Interrupted and friends.

I think we should have flush on a buffered object call write in a loop, propagating the first error it sees. It can keep track of the unwritten data, so that no data is lost and it's possible to separately query -- or even get access to -- the remaining data.

https://github.com/rust-lang/rust/pull/21835#discussion_r23898163

Should flush return Ok by default? We may want to make an explicit decision about the default here (there hasn't been much discussion on the flushing topic)

I think @mahkoh is raising a good point here, and flush should be a required method (no default impl) to help ensure it has a sensible implementation.

https://github.com/rust-lang/rust/pull/21835#discussion_r23898182

A suggestion that write_all should return Result<u64> instead of Result<()>

The proposal in the comment, in particular, says that write_all should report Ok on "EOF" for the underlying writer, but no motivation was given. Keep in mind that the point of write_all is just as a convenience that avoids any looping and gives up on the first non-EINTR error. Yielding Ok on EOF would require extra complexity on the client side, and it's not obvious why you'd want it for this convenience method.

https://github.com/rust-lang/rust/pull/21835#discussion_r23898240

A suggestion to use i64 in SeekFrom::Start instead of u64. Note that this would also raise a question as to why it takes a payload i64 instead of a second argument.

This connects to broader API guidelines, of course, but in general in Rust APIs we use types to convey semantic content/limitations when possible.

re: EINTR handling
I think at a minimum you should LOUDLY document any function that can
return EINTR, considering how many people see that is as a bug not a
feature, and will just use the shorter method name (write is shorter than
write_all).
Also, I think it would be more conservative to make read/write retry EINTR
by default, and if people complain, there could be a backwards compatible
set_retry_interrupt(false) method added to certain objects they care about,
depending on their actual use case.   If instead you make no retry the
default, and that turns out to be a usability mistake, you can't change
that default globally without potentially breaking somebody, and forcing
devs to set an opt-out flag on all io objects in their program, and their
libraries, and those libraries' libraries, etc. will be impossible, just
like the FD_CLOEXEC debacle in posix (that should have been opt in not opt
out)
On Fri, Jan 30, 2015 at 1:25 PM, Aaron Turon notifications@github.com
wrote:

I want to try to summarize the current state here, to help move things
forward.
It seems like there's reasonable consensus around most of the basic
structure here, in terms of slimming down the traits, sticking closer to
single syscall semantics, EOF handling, and so on. I think the few remaning
items should be quickly resolved or pulled out to separate discussions so
we can make progress.
The items still being debated are:
EINTR handling
It's not completely clear when this should just cause a retry. However,
this RFC is mainly covering the core traits for reading and writing, and
not specific APIs. I think our best way forward here is to have the core
read/write operations map as directly to the underlying system APIs as
possible, producing an error on EINTR, so that they can serve as a lossless
building block for other abstractions.
Conveniences like write_all or read_to_end, on the other hand, should
retry on EINTR. This is partly under the assumption that in practice, EINTR
will most often arise when interfacing with other code that changes a
signal handler. Due to the global nature of these interactions, such a
change can suddenly cause your own code to get an error irrelevant to it,
and the code should probably just retry in those cases. In the case where
you are using EINTR explicitly, read and write will be available to
handle it.
read_until's behavior
Does it include the delimiter in the strings it produces (as today's API
does)? I think the right approach is for read_line/read_until to retain
the delimiter, while lines consumes it. This keeps consistency with the
split iterator, and also existing precedent (and useful coding patterns)
for read_line; it seems like the pragmatic solution.
close
The discussion about close is probably more relevant on RFCs for specific
IO APIs.
That said, I believe that close is backwards compatible to add later, and
am personally not yet convinced that it's needed given the existence of
flush. There's now a separate RFC
https://github.com/rust-lang/rfcs/pull/770/ on this topic, and I think
the discussion should move there (but shouldn't block this orthogonal RFC).
stdin/stdout/stderr:
I think this part of the RFC is underdeveloped, and I'd like to have a
separate discussion about it. I propose to remove it from this RFC for now.
Taking &mut Vec
I made a suggestion
https://github.com/rust-lang/rfcs/pull/576#issuecomment-71135228 about
an API change that hasn't yet been incorporated into the RFC. I'll plan to
update the RFC with it, but please give feedback on this idea if you have
any.
Various names:
There are a few open bikesheds here that will need to be decided, but
nothing too major I think.
—
Reply to this email directly or view it on GitHub
https://github.com/rust-lang/rfcs/pull/576#issuecomment-72255620.

At this time it looks like there's broad enough support for this amendment that I'm going to merge it.
There are still some concerns about using a concrete io::Error instead of an associated type, but there are avenues in the future to use a default associated type if necessary and we also have a window of time (albeit small) in the near future to tweak this aspect of the design.
There are also some lingering concerns about EINTR and how it is handled via various primitives. We're taking a relatively conservative stance by propagating the "error" on calls to read and write but auto-retrying on calls such as fsync (for files). We also have some leeway to tweak this in the near future, and it should in theory be a largely backwards compatible extension to start handling EINTR if deemed necessary (as @kjpgit pointed out with Python's pep).
There have also been a number of proposals for backwards-compatible extensions. Due to this compatibility this RFC is going to go ahead and land ahead of them and future RFCs can be used to add new traits and/or methods where the design can be hashed out.
I'd like to again emphasize that none of this functionality will land initially as #[stable] and we still have some time to tweak the design in various places. The overall direction this is headed, however, definitely seems to have broad support.
I'd like to thank everyone again who participated in this thread (and #517!), all the comments have been quite helpful and this is all definitely moving in a great direction. Thank you!

authors: Binero Ericson2314 Hoverbear Marwes Tobba alexcrichton aturon blaenk brson kjpgit m13253 mahkoh netvl nodakai oli-obk pythonesque quantheory retep998 stepancheg tailhook


text:
Custom attributes sometimes not formatted

rustfmt 1.4.5-nightly (2019-08-19 9792ff0)
enum E {
    #[error(display = "invalid max keyframe interval {} (expected > 0, < {})", _0, i32::max_value() as u64)]
    InvalidMaxKeyFrameInterval(u64),
    #[error(display = "invalid max keyframe interval {} (expected > 0, < {})", _0, i32::max_value())]
    InvalidMaxKeyFrameInterval(u64),
}

gets reformatted as
enum E {
    #[error(display = "invalid max keyframe interval {} (expected > 0, < {})", _0, i32::max_value() as u64)]
    InvalidMaxKeyFrameInterval(u64),
    #[error(
        display = "invalid max keyframe interval {} (expected > 0, < {})",
        _0,
        i32::max_value()
    )]
    InvalidMaxKeyFrameInterval(u64),
}

even though this also fits fine
enum E {
    #[error(
        display = "invalid max keyframe interval {} (expected > 0, < {})",
        _0,
        i32::max_value() as u64
    )]
    InvalidMaxKeyFrameInterval(u64),
    #[error(
        display = "invalid max keyframe interval {} (expected > 0, < {})",
        _0,
        i32::max_value()
    )]
    InvalidMaxKeyFrameInterval(u64),
}

Looks like ast::Attribute::meta returns None against the first attribute, and rustfmt cannot process it further.
https://github.com/rust-lang/rustfmt/blob/1ded995ee79c44069b7832e5aba00162fc2113fa/src/attr.rs#L332

confirming this is reproducible with rustfmt 1.5.1-nightly (f2c31ba0 2022-07-19)

ast::Attribute::meta() returns None because it does not handle casting.   I tried to suggest a a fix, but found that there is no way to store the cast information in theast::Attribute structure.  Therefore it seems that the solution is to rewrite ast::Attribute structure directly instead of using ast::Attribute::meta().  I didn't try this approach as it seems to be a big task and I am not sure it is worth the effort.
Just for the record, and maybe for future use, following is what I found.  Inserting the following code after let hi = match kind { ...} in ast::MetaItem::from_tokens() identifies the "as u64" cast tokens, but I didn't find a way to store this information in the ast::MetaItem structure.
// Handle "as"
match tokens.peek() {
    Some (TokenTree::Token(rustc_ast::token::Token { kind: token::Ident(sym, _), span }, _)) => {
        match sym.as_str() {
            "as" => {
                let as_path_segment =  PathSegment::from_ident(Ident::new(*sym, *span));
                let as_token = tokens.next();
                match tokens.peek() {
                    Some (TokenTree::Token(rustc_ast::token::Token { kind: token::Ident(sym_as_unit, _), span }, _)) => {
                        let unit_path_segment =  PathSegment::from_ident(Ident::new(*sym_as_unit, *span));
                        let as_unit_token = tokens.next()?;
                        hi = token.span().hi();

                       //  No way to the add the information fro as_token and as_unit_token into the MetaItem structure ....
                    }
                    _ => {
                        return None;
                    }
                }
            }
            _ => (),
        }
    }
    _ => (),
}

source: gharchive/issue

authors: YaLTeR davidBar-On topecongiro ytmimi

repo: rust-lang/rustfmt

license_type: permissive

license_source: github-api


text:
In workspaces, cargo fmt only works in the top-level directory #8724

Describe the bug

$ cargo fmt
Failed to find targets
cargo-fmt 1.4.18
This utility formats all bin and lib files of the current crate using rustfmt.

USAGE:
    cargo fmt [FLAGS] [OPTIONS] [-- <rustfmt-options>...]

FLAGS:
        --all        Format all packages (only usable in workspaces)
    -q, --quiet      No output printed to stdout
    -v, --verbose    Use verbose output
        --version    Print rustfmt version and exit

OPTIONS:
        --manifest-path <manifest-path>      Specify path to Cargo.toml
        --message-format <message-format>    Specify message-format: short|json|human
    -p, --package <package>...               Specify package to format (only usable in workspaces)

ARGS:
    <rustfmt-options>...    Options passed to rustfmt

To Reproduce

cargo new outer && cd outer
cargo new inner
printf '[workspace]\nmembers = ["inner"]' >> Cargo.toml
cd src
cargo fmt

Expected behavior
cargo fmt should format all packages in the workspace, like it does from the top-level directory.
Meta

rustfmt version: rustfmt 1.4.21-nightly (01f2ead 2020-09-04), but the problem is also present on rustfmt 1.4.18-stable (8157a3f 2020-07-15)
From where did you install rustfmt?: rustup
How do you run rustfmt: cargo fmt

You need to include the --all if you want to run against the entire workspace from a sub directory. Some more info here

It should say that in the error message, then. Right now it just looks like a bug.

It should say that in the error message, then. Right now it just looks like a bug.

I just wanted to clarify that the expected behavior you provided in the issue description was not in alignment with the expected cargo fmt behavior. Full workspace formatting from a subdirectory is supposed to be an opt-in via the flag so that folks have the ability to format just the local member as well as the entire workspace.
That being said, I do agree there's a bug and suspect it's a regression of an older one

@calebcartwright it is unclear to me what is considered the bug in this scenario and what the expected behavior since it sounds like some of the behavior is expected.
Personally, I would expect it to behave like cargo build and when run inside of a src directory, it would format that crate.  Also worth noting that this behavior is different, depending on if you are in a workspace or not.
(came across this issue when looking at #5010)

This also happens if you're not inside of any package inside the workspace, e.g. a .github folder. I would expect that to format the whole workspace since it's unambiguous.

See also #2280.

What are the updates on this?

authors: calebcartwright epage ethanuppal jyn514 sanmai-NL


text:
Support for platforms other than *86

This crate is the shortest path to solving a stack size issue I have, but unfortunately it does not compile for armv7.
Would be great if further platforms are also supported.

This is not an useful issue without more information, especially because we do have code in place to support all sorts of 32-bit ARM implementations that can utilize either the GAS or the armasm assembler.

I'll close this for now. Please file a new issue with more information.

authors: marcbrevoort-cyberhive nagisa

repo: rust-lang/stacker


text:
Consider using route_recognizer instead of path-table

Detailed Description
For now Router is using path-table for its route matching. Consider changing the algorithm by replacing it with route_recognizer.
The published version of route_recognizer is unmaintained for about 2 years, and seems to have reversed precedence of path segment matching (conduit-rust/route-recognizer.rs#21). Maybe we can fork the repository and apply some patches.
Context
Related to #12. route_recognizer seems to have more sophiscated route matching algorithm than path-table. I expect it to fix most of the counterintuitive cases.

There is actix_router, but it is Independent，and dont dependent Actix* . It is General

Note: this is the algorithm ultimately used by crates.io:

https://github.com/conduit-rust/conduit-router
https://github.com/rust-lang/crates.io/blob/master/Cargo.toml#L76

so it's "maintained", just hasn't needed a lot of updating. Definitely a worthy candidate to look at, and I  expect the current owners would be  interested in moving it into the rust-net-web org.

One thing to consider for the router API is a way of providing reverse lookups #24. I'll go into more detail about the small survey I did of current implementation of such an API in django and flask, but both of them leverage the router to provide a reverse lookups of endpoints to paths for building URLs.

So, we need a router that:

has intuitive path matching algorithm
can be nested
has ability to perform reverse lookups

path-table currently used by Tide addresses only the second point, and both route_recognizer and actix-router don't have reverse lookup feature (as far as I understand). I think we need to fork and add needed functionality anyway, so how about experimenting with route_recognizer? It might be challenging with state machines, though.

I wrote a https://github.com/trek-rs/path-tree.

supports reverse lookups

supports named and catch-all likes https://github.com/julienschmidt/httprouter, but more flexible.

Currently, I am thinking about how to generate urls. Maybe the {} is best syntax.

The #156 PR moved to route-recognizer. And in general it should now be much easier to swap out the underlying router. I'm open to moving to path-tree if that seems like a better option -- though offhand the two seem pretty comparable in terms of functionality (and route-recognizer is used by crates.io, so there's some impetus for sharing).
I'm going to close this for now, but if @fundon or others want to explore moving from route-recognizer to path-tree or something else, please feel free to open a new issue and make a case there!

authors: aturon bIgBV fundon krircc tirr-c

repo: rust-net-web/tide


text:
Drop unsafe Send traits on structs including libgpiod structures

A new lint pointed out some weird Arc<Mutex<>> use that turned out to happen because we are (unsafely) extending the contract of libgpiod [1].
The following code is marking PhysDevice as Send:
https://github.com/rust-vmm/vhost-device/blob/802d09c542216b6f7beb1df30563fec5ba942946/crates/gpio/src/gpio.rs#L123-L128
PhysDevice is not Send automatically since PhysLineState contains request::Request which is not Send (because it contains a raw pointer).
Instead of just marking that as Send, the annotation in libgpiod needs to be fixed. Or - if that is not possible - we need to rewrite things so that the Send trait is not needed on PhysDevice. I am not immediately seeing anything that would disallow marking the line_request wrapper as Send, but it needs a discussion on whether libgpiod considers this as contract.
I kicked off a discussion around documenting libgpiod's threading safety guarantees [1]. Until that, we should silence the warning in order to keep CI happy.
[1] https://github.com/rust-vmm/vhost-device/pull/435#issuecomment-1717205620
[2] https://lore.kernel.org/linux-gpio/CVHO091CC80Y.3KUOSLSOBVL0T@ablu-work/
/cc @vireshk

@vireshk when you have time, can you take a look?

I am working on a fix currently. Some cleanup patch is sent already:
https://lore.kernel.org/linux-gpio/20230927-chip-drop-arc-v1-0-28013bf95948@linaro.org/
Found some other issues that I am going through to fix.

Turns out that this is more than a cosmetical marker issue. The (wrong) unsafe Sync impl is allowing API interactions that are unsafe according to libgpiod's threading rules. process_event is sharing a chip instance with other threads -which is unsafe. We will probably need to shuffle some mutexes around.
I will draft a patch.

authors: Ablu stefano-garzarella

repo: rust-vmm/vhost-device


text:
Add PCI configuration space abstractions

Summary of the PR
This PR adds:

common PCI configuration space abstractions
PCI device configuration space abstractions
PCI bridge constants
Base Address Register abstractions
high-level crate documentation

A more detailed view of the PR contents is available in the documentation part.
Requirements
Before submitting your PR, please make sure you addressed the following
requirements:

[ ] All commits in this PR are signed (with git commit -s), and follow
the 50/72 git commit rule.
[ ] All added/changed functionality has a corresponding unit/integration
test.
[ ] Any newly added unsafe code is properly documented.

We have decided to deprioritize the PCI work in Firecracker. While we’re keeping our options open for the future, we’re closing this PR for now.

source: gharchive/pull-request

repo: rust-vmm/vm-pci


text:
OpenBSD does not support extended file attributes

#522 breaks building on OpenBSD because there is no xattr support.

Thanks a lot for your PR! Unfortunately I didn't manage to build OpenBSD in the CI.
If you see a way to add it, please open a PR!
I fixed and reformatted this PR. If the CI compiles it, I'll merge it.

authors: aawsome bket

repo: rustic-rs/rustic


text:
Implement SSL_CONF_*

In https://github.com/rustls/rustls-openssl-compat/pull/20 we stubbed out the SSL_CONF_* API surface used by Nginx. There's a lot of functionality behind these functions, but it looks like it would be important/reasonable to implement support for the options that have obvious rustls config counterparts.

cpu self-assigned this now

I've started to work on this.

We've implemented support for MinProtocol, MaxProtocol, VerifyMode, Certificate, PrivateKey, VerifyCAPath and VerifyCADir.
There are other commands we could implement once additional infrastructure is in place (see this comment) but for now I think it makes sense to call this "Done Enough :tm:" and focus on other areas until there's a reason to re-prioritize.

repo: rustls/rustls-openssl-compat


text:
CollisionWorld::contact_pairs doesn't change after resolving collisions before CollisionWorld::update

I have a short repro:
use nalgebra::{Isometry3, Translation, Vector3, zero};
use ncollide3d::{
    shape::{Cuboid, ShapeHandle},
    world::{CollisionGroups, CollisionWorld, GeometricQueryType},
};
use std::collections::HashMap;

fn two_colliding_cuboids() {
    let mut world = CollisionWorld::new(0.0);

    // Add two intersecting cuboids to the world.
    let mut groups = CollisionGroups::new();
    groups.set_membership(&[1]);
    groups.set_whitelist(&[1]);
    let contacts_query = GeometricQueryType::Contacts(0.0, 0.0);
    let shape = ShapeHandle::new(Cuboid::new(Vector3::new(1.0, 1.0, 1.0)));
    let pos = Isometry3::new(zero(), zero());
    world.add(pos, shape.clone(), groups, contacts_query, 1);
    world.add(pos, shape, groups, contacts_query, 2);

    loop {
        // BUG: updating doesn't clear out the old contact pairs, so in the loop below, we will try
        // pushing apart objects that, according to their positions and shapes, should not be
        // in contact anymore.
        world.update();

        // Resolve intersections by pushing cubes apart along the contact normal.
        let mut move_actions = HashMap::new();
        for (handle1, handle2, _, manifold) in world.contact_pairs(true) {
            // Once a room from a contact has been moved, it may invalidate other contacts that
            // it was a part of, so skip them.
            if move_actions.contains_key(&handle1) || move_actions.contains_key(&handle2) {
                continue;
            }

            let contact = &manifold.deepest_contact()
                .expect("No penetration in contact").contact;

            if contact.depth == 0.0 {
                continue;
            }

            // The normal should be parallel to some axis. To ensure that there are no endless
            // cycles of resolutions, only allow resolutions to push in the positive direction
            // along each axis.
            let n: Vector3<f32> = contact.depth * contact.normal.into_inner();
            let (push_v, move_handle) = if n.x > 0.0 || n.y > 0.0 || n.z > 0.0 {
                println!("Pushing room 1");
                (-n, handle1)
            } else {
                println!("Pushing room 2");
                (n, handle2)
            };

            move_actions.insert(move_handle, push_v);

            let obj1 = world.collision_object(handle1).unwrap();
            let obj2 = world.collision_object(handle2).unwrap();
            println!("C1 = {:?}", obj1.shape().as_shape::<Cuboid<f32>>());
            println!("C2 = {:?}", obj2.shape().as_shape::<Cuboid<f32>>());
            println!("C1 = {:?}", obj1.position().translation.vector);
            println!("C2 = {:?}", obj2.position().translation.vector);
            println!("Depth = {}", contact.depth);
            println!("N = {}", contact.normal.into_inner());
        }

        // Need to perform movements after releasing the borrow on world.
        for (handle, push_v) in move_actions.iter() {
            let move_obj = world.collision_object_mut(*handle)
                .expect("Collision object does not exist for handle");
            let mut pos = move_obj.position().clone();
            pos.append_translation_mut(&Translation::from(*push_v));
            move_obj.set_position(pos);
        }

        if move_actions.is_empty() {
            break;
        }
    }
}

fn main() {
    two_colliding_cuboids();
}

Basically, I'm trying to use a CollisionWorld to push apart a bunch of colliding cuboids. I've simplified the example here to just 2 cubes. You'll see that with ncollide3d = "0.19", the example will loop forever because it never realizes that the cubes are not in contact after being pushed apart.
This must be a regression in behavior according to the documentation of an older version (0.14.1).
It states that contact_pairs "iterates through all the contact pairs detected since the last update." So you can see the problem is that it also iterates through pairs detected before the last update.
I think maybe I can work around this using the contact_events function, but it makes things more complicated because of my particular application. I don't want to push the objects more than necessary, so, for each iteration over the contacts, I will only push an object the first time I see it in a contact pair. By pushing an object, it might resolve more collisions that come up in the contact iterator later, and I don't want to try resolving these unnecessarily. If I have to use contact_events, then when I ignore a "stale" contact, I may never see the contact pair again if the collision is left unresolved. At least this is my interpretation of the docs here. Specifically, this part about ContactEvent::Started: "This event is generated whenever the narrow-phase finds a contact between two collision objects that did not have any contact at the last update."
It would be greatly appreciated if someone could suggest another workaround while this bug is addressed. Thanks!

Hi! Your example terminates after one iteration with the latest version of ncollide (0.20.1). Can you try again after updating your ncollide3d dependency?

That helps, thanks! I can't remember why I took a specific dependency on 0.19, but now I can upgrade.
So the new version doesn't get stuck, but it does give me an assertion at
ncollide3d-0.20.1/src/query/algorithms/epa3.rs:143
I'll make a new issue for that.

authors: bonsairobo sebcrozet

repo: rustsim/ncollide


text:
Link for HtmlCollection.namedItem documentation isn't valid

Describe the Bug
The link to the MDN webdocs for (HtmlCollection.namedItem)[https://docs.rs/web-sys/0.3.46/web_sys/struct.HtmlCollection.html#method.named_item] doesn't point to the correct location.
Steps to Reproduce

Go to '...'
Click on '....'
Scroll down to '....'
See error

If applicable, add a link to a test case (as a zip file or link to a repository we can clone).
Expected Behavior
Should redirect to the correct page
Actual Behavior
Doesn't redirect correctly
Additional Context

Thanks for the report! This is a known issue where we generate URLs without checking their validity. Unfortunately we don't know how best to fix this at this time.

Thanks for the report! This is a known issue where we generate URLs without checking their validity. Unfortunately we don't know how best to fix this at this time.

authors: alexcrichton teymour-aldridge

repo: rustwasm/wasm-bindgen


text:
Running part of your wasm in a web_sys::Worker?

Summary
What is the best architectural approach to have a 'pure' wasm web application that runs part of the calculations in a web worker?
Additional Details
I am still fairly new to Rust and especially web_sys. However I find the concept of wasm really exciting and I am currently trying out to build an easy web app. It needs to react to clicks on a canvas, some buttons etc., but then there are heavier calculations which should run in the background.
Initially, I followed the Wasm Game of Life tutorial and found a good first entry point there. After reading this guide, I designed my app to handle the user interaction part in JS. The background calculation was running actually in the foreground but returned very regularly, giving a false impression of responsiveness.
Next I read about web_sys and especially building wasm apps that can be deployed without a bundler. Now I rewrote the user interaction part almost purely in Rust and web_sys. Knowing that there is probably no native multiprocessing in wasm as of now, web workers are the only alternative.
My dream scenario would be to spawn a web_sys::Worker and hand it a struct with a method or a function callback to run. However it looks like a web worker expects a URL to execute (1, 2). Loading in any local JS via the URL works, but not a wasm module.
If I create a worker in my main wasm app
pub fn setup_worker() {
    Worker::new("worker.js");
    console::log_1(&"Created a new worker".into());
}

and have this minimal worker.js:
import Dummy from './pkg/my_wasm_app.js';

I get the error SyntaxError: import declarations may only appear at top level of a module. This might have JS reasons (not so familiar with JS ^^). Though maybe someone could point me in the right direction? Which one of the following options is possible? And which one is best?

Setup a separate wasm project with target so that I can load an init. I think it is not possible to have two wasm targets in the same cargo.toml so it would be annoying to maintain two project folders for essentially one projects. Also I would need to figure out a better way to load it given that the above approach at least for some imported module was not allowed in the worker.
Pass a struct to a worker that is being created and glue the interaction with Worker::set_onmessage etc. - this seems best to me but I am not sure if it is possible at all.
Setup the worker code in JS (like e.g. here and just call it from web_sys. I don't like this option very much because it would mean having to dig deeper into JS for me.

Are there any good tutorials on this? I know about the Parallel Raytracing example but I could not really extract how to use the web_sys::Worker from it. Also this WASM Fractal Generator seems to have the worker and canvas interaction in JS, so it is not really what I am looking for.
Any hint/explanation would be greatly appreciated!

Alright, thank you any way @alexcrichton :slightly_smiling_face:  I will take a closer look at the raytrace example then.

I could not let it go and finally figured it out! :smiley: One of the main blockers for me was that Firefox does not support importing ES modules in workers - with my limited JS knowledge, it took me a while to nail down the problem as this.
To be able to go back to it, I created four example starting from a very basic example with modules (which only works in Chrome) over using --target no-modules to a minimal 'full' example in which I spawn a web worker with web_sys from Rust and interact with it in a useful way. You can find the example in this repo. Do you think the fourth example could be helpful for others as well @alexcrichton ? Then I would create a PR for wasm-bindgen/examples and put it up for review.

More examples always seems good to me!

Great, I opened a pull request :) Since it is my first contribution to wasm-bindgen, I hope I followed all the conventions that I could deduce from other examples. Would be great if you or someone else could review!

authors: alexcrichton sgasse


text:
U64 does not work in Safari

Describe the Bug
Safari 14 does not support BigUint64Array as mentioned in https://github.com/rustwasm/wasm-bindgen/pull/2441#issuecomment-806737455
Steps to Reproduce
Run any wasm code/test that exposes u64 in safari.
https://github.com/Firaenix/wasm-bindgen-BigUint64Array-repro
Expected Behavior
Should run in Safari (iOS and macOS) as in any other environment.
Actual Behavior
Console error that causes the code not to run.

Safari 15, released in September, added support for BigInt64Array and BigUint64Array on macOS and iOS. It might take a few more months before a majority of users have upgraded.

I don't think that's a satisfying solution, especially when the documentation claims that it should work on Safari.
If we are already providing what is essentially a polyfill for u64, it shouldnt just be good enough to say that we should just wait for users to upgrade.
I'm working on a PR that should hopefully solve this issue for all BigInt supporting browsers

To be clear, I wasn't offering a solution previously, only describing the current state of Safari support. An improved polyfill that handles Safari 14 would be great.

All good, I misinterpreted in that case

I have a branch on my fork which works in Safari and all other browsers that support BigInt:
https://github.com/bsvwasm/wasm-bindgen/tree/universal-ui64-patch

https://github.com/rustwasm/wasm-bindgen/pull/2727

does #3037 fixed this issue?

does #3037 fixed this issue?

Yes.

Hello, https://github.com/rustwasm/wasm-bindgen/commit/bf109de9b2a46de0d8e7ffea198823e5a6744a1b was supposed to fix this issue, but release 0.2.83 still fails upon using BigInt64Array on Safari iOS 14.8. This has been tested through Ruffle and the issue started when they updated to 0.2.83 (https://github.com/ruffle-rs/ruffle/commit/1954f6162f6b53546f216efc0de3d029ea54cc5e). Any ideas?

#3037 fixed the problem for passing i64s and u64s directly, but it turns out that returning Option<i64> or Option<u64> still ends up using BigInt64Array for other reasons.
Right now (since #2886), that BigInt64Array gets initialised immediately, which means that trying to run something that might use it in Safari 14 will fail instantly. I've opened #3253 to change it back to only being initialised on first use, so that things can still work in Safari 14 as long as they never reach the code that uses the BigInt64Array. I think that should be enough to get Ruffle working.

Thanks for your detailed answer :) Unfortunately I cannot test right now because I have no idea how to compile Ruffle from source, even less how to use a specific branch rather than the release they're using. I'll update you ASAP (so whenever the PR is commited and they update their dependencies)

authors: Firaenix Liamolucko dovahcrow memoryruins senolem


text:
At least for impl blocks, #[wasm_bindgen] can not be applied to paths

In other words, #[wasm_bindgen] only works in impl blocks if the target element is an identifier.
#[wasm_bindgen]
struct Foo;

#[wasm_bindgen]
impl self::Foo { // Error! Should also work for paths
    #[wasm_bindgen]
    pub fn foo() {
    }
}

Feel free to close this issue if duplicated.

Closing due to the lack of activity


text:
guide: bump required nodejs version to 10

Fixes confusions such as #465

Sorry about the confusion, and thanks!

authors: alexcrichton jrakow


text:
"wasm-pack new -m no-install foo" failed

🐛 Bug description
Describe your issue in detail.
"wasm-pack new -m no-install foo" failed.
"cargo-generate" is already installed.
"wasm-pack new" command has found "cargo-generate".
but, "wasm-pack new" command overlooks cargo-generate version.
and "wasm-pack new" command reports "Not able to find".
$ wasm-pack new -m no-install foo

 INFO 2022-07-20T17:51:11Z: wasm_pack::child: Running "C:\\Dev\\Rust\\.cargo\\bi
n\\cargo-generate.EXE" "--version"
 INFO 2022-07-20T17:51:11Z: wasm_pack::install: Checking installed `cargo-genera
te` version == expected version: generate == 0.15.2

Error: Not able to find or install a local cargo-generate.

https://github.com/rustwasm/wasm-pack/blob/e7d57690b47f8575dfbf0dd9a0f04539d0af4936/src/install/mod.rs#L101-L106
https://github.com/rustwasm/wasm-pack/blob/e7d57690b47f8575dfbf0dd9a0f04539d0af4936/src/install/mod.rs#L109-L119
https://github.com/rustwasm/wasm-pack/blob/e7d57690b47f8575dfbf0dd9a0f04539d0af4936/src/install/mod.rs#L114
cargo-generate --version output cargo generete 0.15.2
cargo generate 0.15.2 nth(1) is generate.
 INFO 2022-07-20T17:51:11Z: wasm_pack::install: Checking installed `cargo-genera
te` version == expected version: generate == 0.15.2

🤔 Expected Behavior
What should have happened?
success of "wasm-pack new -m no-install foo"
👟 Steps to reproduce
Clear steps describing how to reproduce the issue, including commands and flags run. If you are seeing an error, please include the full error message and stack trace.
C:\temp\RustProjects
$ cargo-generate --version
cargo generate 0.15.2

C:\temp\RustProjects
$ wasm-pack new -m no-install foo
 INFO 2022-07-20T17:51:10Z: wasm_pack::command: Running generate command...
 INFO 2022-07-20T17:51:10Z: wasm_pack::command: Template: "https://github.com/ru
stwasm/wasm-pack-template"
 INFO 2022-07-20T17:51:10Z: wasm_pack::command: Name: "foo"
 INFO 2022-07-20T17:51:10Z: wasm_pack::command::generate: Generating a new rustw
asm project...
 INFO 2022-07-20T17:51:11Z: wasm_pack::child: Running "C:\\Dev\\Rust\\.cargo\\bi
n\\cargo-generate.EXE" "--version"
 INFO 2022-07-20T17:51:11Z: wasm_pack::install: Checking installed `cargo-genera
te` version == expected version: generate == 0.15.2
[INFO]: Installing cargo-generate...
 WARN 2022-07-20T17:51:11Z: wasm_pack::install: could not download pre-built `ca
rgo-generate`: no prebuilt cargo-generate binaries are available for this platfo
rm: Unrecognized target!. Falling back to `cargo install`.
Error: Not able to find or install a local cargo-generate.

C:\temp\RustProjects
$ rustup show
Default host: i686-pc-windows-msvc
rustup home:  C:\Dev\Rust\.rustup

installed targets for active toolchain
--------------------------------------

i686-pc-windows-msvc
wasm32-unknown-unknown

active toolchain
----------------

stable-i686-pc-windows-msvc (default)
rustc 1.62.0 (a8314ef7d 2022-06-27)

C:\temp\RustProjects
$ wasm-pack --version
wasm-pack 0.10.3

wasm-pack new -m no-install foo is
cargo-generate --git https://github.com/rustwasm/wasm-pack-template --name foo
OK, I use cargo-generate --git https://github.com/rustwasm/wasm-pack-template --name foo
resolve
https://github.com/rustwasm/wasm-pack/blob/e7d57690b47f8575dfbf0dd9a0f04539d0af4936/src/generate.rs#L18-L21

repo: rustwasm/wasm-pack


text:
XConv: knn_graph() got an unexpected keyword argument 'num_workers'

🐛 Bug

To Reproduce
Steps to reproduce the behavior:

Define an XConv layer
https://github.com/rusty1s/pytorch_geometric/blob/04143195eaa52dcfa56e8656e72df57c36a76367/benchmark/points/point_cnn.py#L24

The forward operation on this layer throws the error
https://github.com/rusty1s/pytorch_geometric/blob/04143195eaa52dcfa56e8656e72df57c36a76367/benchmark/points/point_cnn.py#L37

where pos.shape == torch.Size([960000, 3]) and batch.size = 960000 (tensor([0, 0, 0, ..., 63, 63, 63])).
Error sessage:
...
  File "...\modelPCR.py", line 112, in forward
    x = self.conv1(None, pos, batch)
  File "...\lib\site-packages\torch\nn\modules\module.py", line 532, in __call__
    result = self.forward(*input, **kwargs)
  File "...\lib\site-packages\torch_geometric\nn\conv\x_conv.py", line 127, in forward
    num_workers=self.num_workers)
TypeError: knn_graph() got an unexpected keyword argument 'num_workers'

Process finished with exit code 1

Expected behavior
No error.
Environment

OS: Win10
Python version: 3.6.10
PyTorch version: 1.4.0
CUDA version: 10.1
torch-geometric: 1.6.1
torch-cluster: 1.5.4
torch-scatter: 2.0.4
torch-sparse: 0.6.1
torch-spline-conv: 1.2.0

Downgrading torch-geometric to 1.5.0 solved the problem.

Perfect :)

license: MIT