Complicated ownership using AnyReader

[bibo38]

Zig version: 0.12.0-dev.800+a9b37ac63

Just stumbled upon the new type erased std.io.AnyReader and wanted to use it to write generic code. But I quickly wrote some buggy code, as shown in this example:

// test.zig
const std = @import("std");
const fs = std.fs;
const io = std.io;

const CoolReader = struct {
    file: fs.File,

    fn init() !CoolReader {
        return CoolReader{ .file = try fs.cwd().openFile("test.zig", .{}) };
    }

    fn reader(self: *CoolReader) io.AnyReader {
        return self.file.reader().any();
    }
};

pub fn main() !void {
    var rd = try CoolReader.init();
    _ = try rd.reader().readInt(u32, .Little);
}

Running this with zig run test.zig caused some undefined behaviour, as the program reads the standard input instead of it's own sourcefile.

The problem is that the reader returned from self.file.reader() is discarded at the end of the function, but the returned io.AnyReader contains a pointer to it:

zig/lib/std/io.zig

Lines 336 to 341 in 75b48ef

	pub inline fn any(self: *const Self) AnyReader {
	return .{
	.context = @ptrCast(&self.context),
	.readFn = typeErasedReadFn,
	};
	}

As the reader() function copies the File struct into the context of the returned GenericReader, I instead stored this reader and accessed the context variable, if I needed the File struct.

But this pattern didn't work as I introduced a io.CountingReader, as it uses a pointer context for it's reader to update the struct values:

zig/lib/std/io/counting_reader.zig

Line 12 in 75b48ef

pub const Reader = io.Reader(*@This(), Error, read);

This meant that I have to store the io.CountingReader for the actual counter, but also the io.CountingReader.Reader struct to be able to return an io.AnyReader without undefined behaviour. But theoretical we could copy the context pointer instead of a double indirection.

TLDR: It's just complicated to use AnyReader, as you need to store the underlying reader. But without knowing the implementation details, you also need to store the actual struct the reader is based upon. This makes it hard to use and can cause unexpected undefined behaviour.

Possible solutions:

• Add an anyReader function, that directly returns an AnyReader and remove the any() function of the GenericReader
  • Good compability with existing code
  • More code necessary to support reader() and anyReader()
• Always use a pointer context in GenericReader
  • any() now just copies the pointer into AnyReader
  • It's clear, that any reader just contains a reference to the actual struct.
  • May break some existing code, that assumes that reader() creates a copy

[andrewrk]

I should make that any function non-public. It doesn't really make sense to convert from a generic reader to a type-erased reader. You need to create the type-erased reader directly, like this:

    fn reader(resource: *Resource) std.io.AnyReader {
        return .{
            .context = resource,
            .readFn = read,
        };
    }

    fn read(context: *const anyopaque, buffer: []u8) anyerror!usize {
        const resource: *Resource = @constCast(@ptrCast(@alignCast(context)));
        switch (resource.*) {
            .file => |*f| return f.read(buffer),
            .http_request => |*r| return r.read(buffer),
            .git => |*g| return g.fetch_stream.read(buffer),
            .dir => unreachable,
        }
    }

[bibo38]

Is it intended that the stdlib also provides a way to get an AnyReader from all the structs that currently return a GenericReader (matching my first possible solution)?
I personally like the ability to just pass them around and assign them instead of creating custom Multiplexer Unions (your example) or using anytype to accept a variety of readers as function arguments:

fn parse(self: *@This(), rd : io.AnyReader) !void {
    var zlibDec : zlib.DecompressStream(io.AnyReader) = undefined;
    if(try rd.readInt(u32, .Little) == 1) {
        zlibDec = zlib.decompressStream(self.alloc, rd);
        rd = zlibDec.anyReader();
    }
    const num1 = try ComplexNumber.parse(rd)
    // ...
}