WIP Gap Buffer in Zig: Looking for Feedback

Hello Everyone,

I’m new to Zig and coming from higher‑level languages. To learn Zig, especially its memory management and idioms, I wrote a (buggy) gap buffer implementation. The code is below. I’d appreciate any feedback on Zig idioms, memory management, performance, algorithm correctness, and error handling, since that’s an area I’m still adjusting to after working with higher‑level languages.

Cheers.

const std = @import(“std”);
const Allocator = std.mem.Allocator;
const print = std.debug.print;

const GapBuffer = struct {
    allocator: Allocator,
    data: u8,
    gap_start: usize = 0,
    gap_end: usize,
    capacity: usize,

    pub fn init(allocator: Allocator, capacity: usize) !GapBuffer {
        return .{
            .allocator = allocator,
            .data = try allocator.alloc(u8, capacity),
            .gap_end = capacity - 1,
            .capacity = capacity,
        };
    }

    pub fn deinit(self: *GapBuffer) void {
        self.allocator.free(self.data);
    }

    pub fn insert(self: *GapBuffer, char: u8) !void {
        if (self.gap_end == self.gap_start) {
            try self.grow();
        }

        self.data[self.gap_start] = char;
        self.gap_start += 1;
    }

    pub fn delete(self: *GapBuffer) !void {
        if (self.gap_end == self.data.len - 1) {
            return;
        }

        self.data[self.gap_end + 1] = undefined;
        self.gap_end += 1;
    }

    pub fn backspace(self: *GapBuffer) !void {
        if (self.gap_start == 0) {
            return;
        }

        self.data[self.gap_start - 1] = undefined;
        self.gap_start -= 1;
    }

    pub fn move_cursor_forward(self: *GapBuffer) !void {
        if (self.gap_start == self.data.len - 1) {
            return;
        }

        if (self.gap_end < self.gap_start) {
            try self.grow();
        }

        const char = self.data[self.gap_start + 1];
        self.data[self.gap_start + 1] = self.data[self.gap_end];
        self.data[self.gap_end] = char;

        self.gap_end += 1;
        self.gap_start += 1;
    }

    pub fn move_cursor_backward(self: *GapBuffer) !void {
        if (self.gap_start == 0) {
            return;
        }

        if (self.gap_end < self.gap_start) {
            try self.grow();
        }

        const char = self.data[self.gap_start - 1];
        self.data[self.gap_start - 1] = self.data[self.gap_end];
        self.data[self.gap_end] = char;

        self.gap_end -= 1;
        self.gap_start -= 1;
    }

    pub fn grow(self: *GapBuffer) !void {

        // alloc new capacity
        const new_data = try self.allocator.alloc(u8, self.data.len + self.capacity);

        // copy the left of the gap
        // memory: [A][B][C][][X][Z], collect [A][B][C]
        const left_data: []u8 = self.data[0..self.gap_start];
        @memcpy(new_data[0..self.gap_start], left_data);

        const right_data_start = self.gap_end + 1;
        const right_data_end = self.data.len - 1;

        var right_data: []u8 = &.{};
        if (right_data_start <= right_data_end) {
            right_data = self.data[right_data_start .. right_data_end + 1];
        }

        // memory: [A][B][C][][X][Z], collect [X][Z]
        // copy right of the gap

        const new_right_data_start = new_data.len - right_data.len;
        @memcpy(
            new_data[new_right_data_start..],
            right_data,
        );

        self.data = new_data;
        self.gap_end += self.capacity;
    }
};

test "test gapp buffer with different operations" {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};

    const allocator = gpa.allocator();
    const capacity: usize = 3;
    var gap_buffer = try GapBuffer.init(allocator, capacity);

    const max = 3;

    var insert_times: u8 = max;
    for (0..insert_times) |i| {
        try gap_buffer.insert(@intCast(97 + i));
    }
    print("INSERT {d}      : {s}\n", .{ max, gap_buffer.data });
    // data: abc¬¬¬

    var left_shift: u8 = max - 1;
    for (0..left_shift) |_| {
        try gap_buffer.move_cursor_backward();
    }
    print("LEFT {d}        : {s}\n", .{ left_shift, gap_buffer.data });

    insert_times = max + 5;
    for (0..insert_times) |i| {
        try gap_buffer.insert(@intCast(65 + i));
    }
    print("INSERT {d}      : {s}\n", .{ insert_times, gap_buffer.data });

    left_shift = max - 1;
    for (0..left_shift) |_| {
        try gap_buffer.move_cursor_backward();
    }
    print("LEFT {d}        : {s}\n", .{ left_shift, gap_buffer.data });

    const right_shift = max;
    for (0..right_shift) |_| {
        try gap_buffer.move_cursor_forward();
    }
    print("RIGHT {d}       : {s}\n", .{ right_shift, gap_buffer.data });

    var delete_times: u8 = 1;
    for (0..delete_times) |_| {
        try gap_buffer.delete();
    }
    print("DELETE {d}      : {s}\n", .{ delete_times, gap_buffer.data });

    delete_times = 2;
    for (0..delete_times) |_| {
        try gap_buffer.backspace();
    }
    print("BACKSPACE {d}   : {s}\n", .{ delete_times, gap_buffer.data });

    left_shift = 3;
    for (0..left_shift) |_| {
        try gap_buffer.move_cursor_backward();
    }
    print("LEFT {d}        : {s}\n", .{ left_shift, gap_buffer.data });

    try gap_buffer.insert(' ');
    try gap_buffer.insert('1');
    print("INSERT {d}      : {s}\n", .{ insert_times, gap_buffer.data });
}


apologies for lack of comments.

2 Likes

I see a couple of things right off. Your call to allocator.alloc() tells me that where you have data: u8 you really want a pointer, not a single u8. Something like data: []u8 = undefined,. That would properly hold your alloc() result. Second, it’s more idiomatic to NOT store allocator as a member of your struct, but require your caller to provide it in functions that do allocations. In your case, you have many functions that don’t do allocation, but grow() would be a candidate - I’d expect pub fn grow(self: *GapBuffer, gpa: Allocator) as the function signature. This would tell me, the caller, that the function could do some allocating. So many other functions would NOT, and this is nice to be able to see right away in the function signatures. Other functions, like move_cursor_backward()(which might want to be called moveCursorBackward() if you want to follow the typical style), COULD result in an allocation (a call to grow(), in fact), so it would also take an allocator, indicating such. Obviously, what you’ve done isn’t “wrong” - it’s often called a “managed” approach; “unmanaged” approaches are slightly favored, methinks.

Looking a little more… I don’t know much about gap buffers, but

self.data[self.gap_end + 1] = undefined;

seems curious to me. Are you meaning to use undefined as a sentinel? I don’t see it “checked”, later, so perhaps not, but it seems more normal to me to set truly un-initialized data to undefined; once it’s in use, it takes on real values from then on. I don’t think you can expect a particular value, when you assign undefined - I think values get filled to some pattern to help the compiler (or linker or …?) detect, e.g., (unwanted) ‘use of undefined data’.

(I’m new to zig, too… so others may trump me.)

1 Like

Checking the value of undefined is illegal behavior. If you want a sentinel u8 value you need to nominate some legal value of u8 to be the sentinel. In an editor, it’s best not to depend on special values for the data type.

In a gap buffer, moving the cursor should never allocate because it should not be adding any data to the buffer. At the most, it may cause some data to be shifted, but even that is better done only at the moment data is to be added. The only time allocation is needed is when data being inserted is larger than the current gap. That’s partly the point of a gap buffer: to make allocations less frequent.

1 Like

It’s a good idea to use std.testing.allocator in tests because you get leak detection and other debug checks. This will give you better coverage in general, and will report that you forgot to call deinit in the test.

1 Like

it’s not updated post … idk 0.13? but you might look at my gap buffer implementation from 2024

1 Like