Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rework I/O stream abstractions #4710

Merged
merged 16 commits into from
Mar 11, 2020
Merged

rework I/O stream abstractions #4710

merged 16 commits into from
Mar 11, 2020

Conversation

andrewrk
Copy link
Member

The main goal here is to make the function pointers comptime, so that we
don't have to do the crazy stuff with async function frames.

Since InStream, OutStream, and SeekableStream are already generic
across error sets, it's not really worse to make them generic across the
vtable as well.

See #764 for the open issue acknowledging that using generics for these
abstractions is a design flaw.

See #130 for the efforts to make these abstractions non-generic.

This commit also changes the OutStream API so that write returns
number of bytes written, and writeAll is the one that loops until the
whole buffer is written.

The main goal here is to make the function pointers comptime, so that we
don't have to do the crazy stuff with async function frames.

Since InStream, OutStream, and SeekableStream are already generic
across error sets, it's not really worse to make them generic across the
vtable as well.

See #764 for the open issue acknowledging that using generics for these
abstractions is a design flaw.

See #130 for the efforts to make these abstractions non-generic.

This commit also changes the OutStream API so that `write` returns
number of bytes written, and `writeAll` is the one that loops until the
whole buffer is written.
@andrewrk andrewrk added the breaking Implementing this issue could cause existing code to no longer compile or have different behavior. label Mar 10, 2020
@andrewrk
Copy link
Member Author

With these changes, here's an example of what reading from buffered stdin looks like:

 const std = @import("std");
 
 pub fn main() anyerror!void {
-    var stdin_unbuf = std.io.getStdIn().inStream();
-    // damn that is pretty painful, isn't it?
-    const in = &std.io.BufferedInStream(@typeOf(stdin_unbuf).Error).init(&stdin_unbuf.stream).stream;
+    const in = std.io.bufferedInStream(std.io.getStdIn().inStream()).inStream();
 
     var sum: u64 = 0;
     var line_buf: [50]u8 = undefined;

@andrewrk
Copy link
Member Author

@mlarouche - in ed13cff I attempted to fix the ELF emit raw code, but I didn't test it yet. Want to give it a lookover (and possibly submit a fix against this branch?) otherwise, I'll try to run zig build on https://github.com/wendigojaeger/ZigGBAHelloWorld to test it tomorrow before merging this branch.

@mlarouche
Copy link
Contributor

@mlarouche - in ed13cff I attempted to fix the ELF emit raw code, but I didn't test it yet. Want to give it a lookover (and possibly submit a fix against this branch?) otherwise, I'll try to run zig build on https://github.com/wendigojaeger/ZigGBAHelloWorld to test it tomorrow before merging this branch.

I fixed those compile errors when trying to branch (see attached patch) but emit_raw is still completly broken, I don't have time until tonight to fix it properly so feel free to fix it during the day

0001-Fix-compile-with-emit_raw.txt

@LemonBoy
Copy link
Contributor

There are a few compilation errors, a few typos (shdr vs phdr), the code is corrupting section_headers while iterating over it...

@andrewrk
Copy link
Member Author

Notable in this branch is the new std.io.StreamSource abstraction, which is a non-generic data type that can be used to abstract across both memory buffers and files. Usage example here: zigimg/zigimg#41

@mlarouche
Copy link
Contributor

mlarouche commented Mar 11, 2020

Confirmed that latests changes work with ZigGBA

@mlarouche
Copy link
Contributor

Also isn't #130 to make those interface generics instead ?

@andrewrk
Copy link
Member Author

nah, the whole point of a vtable language feature is to provide an interface type which is not generic, but can be used in different contexts with different implementations

@andrewrk andrewrk merged commit 895f67c into master Mar 11, 2020
@andrewrk andrewrk deleted the io-stream-iface branch March 11, 2020 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants