Avoiding duplication of code for readonly or mutable access

I am having some trouble creating functions returning const or mutable stuff without duplicating code. This affects the way to write iterators as well.
Let’s take a simplified image example.
Sometimes I want readonly access, sometimesj mutable access.
How to handle this?
And while writing this thread I was wondering about the 3d function. Is that legal, the image being const and returning a mutable slice?

pub const Image = struct {
    width: u32,
    height: u32,
    pixels: []Pixel,

    // readonly
    pub fn get_line_slice_1(self: *const Image, y: u32) []const Pixel {
        const offset: u32 = self.width * y;
        return &self.pixels[offset..offset + width];
    }

    // mutable
    pub fn get_line_slice_2(self: *Image, y: u32) []Pixel {
        const offset: u32 = self.width * y;
        return &self.pixels[offset..offset + width];
    }

    // what about this??
    pub fn get_line_slice_3(self: *const Image, y: u32) []Pixel {
        const offset: u32 = self.width * y;
        return &self.pixels[offset..offset + width];
    }

}
1 Like

In this case, we only need get_line_slice_3 because the pixels field is just a slice (pointer).

*const Image only prevents modifying the Image object itself through that pointer. Constness is not transitive through the pointer stored in the pixels field, so it is perfectly valid for get_line_slice_3 to return a mutable []Pixel .

Of course, if pixels were an array instead of a slice, things would get a lot trickier.

4 Likes

But that’s why it’s an issue. For array field just a little meta programming on the input constness is enough. Here the problem is how should I encode constness for my “slice wrapper” type.

Zig has power here that library authors don’t have, because []u8 silently casts to []const u8, while MyMutSlice wont cast to MyConstSlice

FWIW here is what I ended up doing zml/zml/slice.zig at 053577d82d702225499d4717b25af3c584608cce · zml/zml · GitHub

Because here, mutable should be determined at compile time and not change at runtime, so encoding it at runtime isn’t really appropriate.

pub const slice = struct {
    pub fn Wrapper(comptime mutable: bool) type {
        return struct {
            bytes: if (mutable) []u8 else []const u8,
            shape: Shape,
            offset_bytes: usize = 0,
            byte_strides: stdx.BoundedArray(i64, constants.MAX_RANK),
            pub fn data(self: @This()) if (mutable) []u8 else []const u8 {
                return self.bytes;
            }
        };
    }
    pub fn initWrapper(shape: Shape, bytes: anytype) Wrapper(!@typeInfo(@typeOf(bytes)).pointer.is_const) {
        return .{
            .bytes = bytes,
            .shape = shape,
            .offset_bytes = 0,
            .byte_strides = shape.computeByteStrides(),
        };
    }
    ...
};

I get it but it makes all API really awkward because of the absence of conversion between const/mut variant

I know that this is just an example, but the correct answer here is don’t use a function when a field access suffices.

2 Likes

The way I usually handle this is by only writing the const version of the function. Then, you can handle the mutable case by using @constCast at the callsite. The rule here is that if you pass in mutable data, then you’re allowed to use @constCast on whatever you get back.

I am going to check on this one… This seems the most logical to me.

I tried your approach at some point, but in the end I decided it wasn’t worth the hassle. In practice const/non constness is relatively easy to track down, compared to eg ownership where the type system don’t help. We already have type safety to track host memory vs accelerator memory, so I didn’t want to multiply the nber of combintions.

1 Like

I recall that in stdlib they are doing some sort of dependent types on Slice.

If Slice is const then result is const, sort of that

Stolen from stdlib: zml/stdx/bounded_array.zig at afb3559f7fd8ca4907ae5c9c37f03d326767c51b · zml/zml · GitHub

(it has been removed from it)

2 Likes

True. I saw that one. It’s ugly because it uses anytype.

IMHO a self: anytype is vastly different to a different to an arg: anytype, because in normal code the . operator ensures it is constrained already and ZLS handles it fine.

self: anytype is also the only way to make your method receiver alignment-generic besides constness-generic.

in a few project I’ve used this

fn IsConstPtr(comptime T: type) bool {
    return @typeInfo(T).pointer.is_const;
}

fn ConstIf(comptime is_const: bool, comptime T: type) type {
    return if (is_const) const T else T;
}

and then you can have :

pub fn getLineslice(self : anytype, y: u32) []ConstIf(IsConstPtr(@TypefInfo(self)),Pixel) {
    const offset : u32 = self.width * y;
    return &self.pixels[offset + self.width];
}

and then at the call site it’s just:

var image: Image = undefined;

const a: []Pixel = image.getLineSclice(0);

const const_image: *const Image = ℑ
const b: []const Pixel = const_image.getLineSlice(0);
2 Likes

Thanks. That is about the best trick possible, I believe.
But it becomes kinda unreadable / tricked in my view. I stick with this simplicity.

pub fn get_line(self: *const Image) []const Pixel {
}
pub fn get_line_mut(self: *Image) []Pixel {
}

Yeah I agree, technically that’s not my favorite solution either, nowadays I prefer to have dedicated “View” inner types, and I usually try to operate on group of things, and therefore this isn’t necessary, but yeah not really readable with the comptime wizardry, although you could remove the function signature typeinfo and have it inside the isConstPtr

I noticed that the abandoned bounded_array std solution also confuses ZLS. It does not know the output type when looping through the slice.

        /// View the internal array as a slice whose size was previously set.
        pub fn slice(self: anytype) switch (@TypeOf(&self.buffer)) {
            *align(alignment.toByteUnits()) [buffer_capacity]T => []align(alignment.toByteUnits()) T,
            *align(alignment.toByteUnits()) const [buffer_capacity]T => []align(alignment.toByteUnits()) const T,
            else => unreachable,
        } {
            return self.buffer[0..self.len];
        }

Still I still think it is an ugly hack and feels like a missing feature in the language.

Actually the first real missing feature I encountered up until now.
This should be read as a compliment :slight_smile:

Are there more places in std where this is done?
Don’t other programmers run into this “problem”?

Yes I just stumbled upon this exact problem and ended up with two versions of the function for simplicity and because I only needed one function to be duplicated. Might look into more complex solutions if there are more functions showing up

I ended up doing the same for clarity.