Better function signatures

Brainstorming
1

This is very much an invitation for brainstorming. In the spirit of Cunningham’s Law, I’m hoping my initial half-baked idea can be improved upon because I do think there is some sort of enhancement that can be done to help authors reduce the cognitive overhead associated with finding the right function to call and using it correctly.

What motivated this post in part was the functions in std.array_list. There is a kind of ad-hoc structure to them which is encoded directly in the function names and wasn’t intuitive to me until I had experience using the various functions. Some of this overhead is to be expected, but I think perhaps some functions could be improved by having more structured signatures that can separate contractual properties / side effects from the simple description of what the function is intended to do.

For example, in the following appendSlice states in its comment that it may invalidate pointers (due to allocation) and appendSliceAssumeCapacity encodes in its name that it assumes capacity is present (and therefore presumably won’t allocate).

/// Append the slice of items to the list. Allocates more
/// memory as necessary.
/// Invalidates element pointers if additional memory is needed.
pub fn appendSlice(self: *Self, items: []const T) Allocator.Error!void

/// Append the slice of items to the list.
/// Never invalidates element pointers.
/// Asserts that the list can hold the additional items.
pub fn appendSliceAssumeCapacity(self: *Self, items: []const T) void

It’s true that an assert is present to check the capacity and that we can also see the absence of an Allocator.Error in the signature, but I think something like the following helps make it clear that there are a subset of functions that share this particular property. Furthermore, the function names themselves can still be simple strings via concatenation and static analysis tools could pick up these ‘tags’ and enforce their contract, etc.

pub fn appendSlice(self: *Self, items: []const T) Allocator.Error!void
pub fn appendSlice[NoAlloc](self: *Self, items: []const T) void

Calling the structured function could be either of the following:

self.appendSlice[NoAlloc](items)
self.appendSliceNoAlloc(items)

Another example might be:

self.foo[NoAlloc, ThreadSafe](items)
self.fooNoAllocThreadSafe(items)

Or

self.foo[NA, TS](items)
self.fooNATS(items)

It might be necessary to declare these at the top level, such that they can also be documented.

/// No allocation takes place.
fn_tag NoAlloc;
/// Safe to call from multiple threads.
fn_tag ThreadSafe;

These are just my initial thoughts. I’m interested to know what you think.

1 Like
2

For the specific case of [NoAlloc] tags, I believe not allowing the function to return an error (or at least not error.OutOfMemory) is enough to statically prove you do not allocate. It may not cover using a static buffer for all allocation though.

Zig langage proposal are not accepted anymore; if you need some sort of tagging system doc comment or just comments might be enough for the static analyzer.

2 Likes
3

I think, for the [NoAlloc] tag it is really hard to even define what counts as allocation. Zig is especially loose in that regard.

E.g. a function could internally use a FixedBufferAllocator on some stack buffer, without you being able to observe any negative effects of it externally.
If you count every accesses to the Allocator interface as allocations, then this use case would not be allowed to be marked as NoAlloc, despite the fact that it is clearly only using stack resources.

On the other hand, there are also people like me who define their own variant of the allocator interface instead of using the one from the standard library, would this mean I could mark all my functions as NoAlloc?

1 Like
4

To me this seems dangerously close to C function overloading / C++ keyword spam.
In Zig, we assume that a function is going to be preceded by few or no keywords, and that a function that does something different is going to have a different name.

That said, it’s not like it’s impossible to do something like you want in Zig. It wouldn’t even be impossible with the current language features.

Firstly, you could use namespacing in a struct. It wouldn’t be hard to, for instance, put appendSlice() in the top-level declaration, rename appendSliceAssumeCapacity() to appendSlice() and put it in a “noalloc” child namespace.
The reason why I wouldn’t recommend doing this is because it’s even more confusing - you’re purposefully using a redundant name between a parent and child namespace for two functions that do have different behaviour.

Another solution is to have a comptime “behaviour” enum that you pass into the function, and change the function’s argument types and return types by calling comptime functions based on this behaviour enum, and change the function’s behaviour by switching based on the behaviour enum.
This is, as far as the compiler is concerned, exactly the same as making two different versions of the function.
You could also make this enum into a bitflags struct, so you can have multiple tags expressed at once like in your proposal.
This way of doing it strikes me as more “Zig”, but I’m kind of neutral towards it.
I wouldn’t necessarily be angry if I saw a library that chose to structure itself in this way, but I think I would be angry if it became some kind of universal standard for the language.

2 Likes
5

I think the simplest thing is to combine 2 conventions in function signatures.

#1 We should use API friction to guide people towards simpler usage of the API where possible, using the simplest, shortest, and most direct name for the simplest version of the function with the least side effects.

append/appendGrow instead of appendAssumeCapacity/append

Redundant names matter less when the function with the simplest name is the one we always reach for, and when the convention is consistent across the ecosystem,

#2 Sort function arguments by increasing frequency of change:

runEventLoop(io: *Io, alloc: *Allocator, parameters: Parameters) void

append(item: T) void
appendGrow(allocator: *Allocator, item: T) !
appendSlice(items: []T) void
appendSliceGrow(allocator: *Allocator, items: []T) !

This makes repeated calls easier to parse, because the parts that don’t change as much blend in with the function name.

array.appendGrow(arena, "Header");
array.appendGrow(arena, body());
array.appendGrow(arena, "Footer");

This also makes it easier to understand the effects of a function by seeing that it performs Io or Allocations right next to the name of the function. This also naturally sorts comptime variables to the front since they conceptually don’t change at runtime.

This originally comes from the functional programming community where function currying implies an ordering to function arguments, but I personally find it very useful in basically every language when reading left to right.

6 Likes
6

Great feedback here, thanks. The main takeaway I’m seeing is that if we want to improve readability while preserving the simplicity of the Zig language itself (which for various reasons we do), then the appropriate alternative is to improve the standard of idiomatic Zig. If I’m not mistaken, this has been the case with Golang too.

1 Like
7

I’ve been experimenting with an alternative implementation and API for ArrayList (among others) in this repo:

https://github.com/oliver-giersch/collections-zig/blob/35c12823fa6bc89a96d4f57adef3e5fc9b0b1ced/src/array_list.zig

Personally, my main gripes with the ArrayList API is just the huge number of largely superfluous methods with horrendously verbose names. For instance, there are 12 methods named append[...] alone and another 12 named add[...].

Anything like appendNTimes (3 methods) should just be removed. This is just become something like:

const slice = try list.appendSlice(allocator, 4);
@memset(slice, value);

All [...]AsArray methods should also go, better to have a one liner at the callsite than 3 extra methods:

const array: *[4]i32 = (try list.appendSlice(allocator, 4))[0..4];

I would also like to see a split into ArrayList and BoundedArrayList, where ArrayList basically becomes a wrapper for BoundedArrayList with an API that takes care of allocations. This would take care of the [...]Bounded methods and possible also of the [...]AssumeCapacity.

So this:

var list: ArrayList(i32) = .empty;
try list.ensureTotalCapacity(allocator, 4);
list.appendAssumeCapacity(1);
list.appendAssumeCapacity(2);
list.appendAssumeCapacity(3);
list.appendAssumeCapacity(4);

Would become:

var list: ArrayList(i32) = .empty;
try list.ensureTotalCapacity(allocator, 4);
list.bounded.append(1) catch unreachable;
list.bounded.append(2) catch unreachable;
list.bounded.append(3) catch unreachable;
list.bounded.append(4) catch unreachable;

try testing.expectEqualSlices(i32, &.{1, 2, 3, 4}, list.bounded.items);

I guess this would be controversial, because the second variant is ultimately more verbose, though, but I personally prefer the greater explicity and smaller, more uniform API surface.

So instead of a total of 24 append or add style methods as we have now, we could do with a total of 16, split evenly among BoundedArrayList and ArrayList:

For returning an uninitialized single-item pointer or slice:

  • appendAt
  • append
  • appendSliceAt
  • appendSlice

For directly inserting an item:

  • pushAt
  • push
  • pushSliceAt
  • pushSlice
2 Likes
8

I agree there’s too much going on in the ArrayList API - keep in mind though a drawback of reimplementation is that unless it’s upstreamed, you potentially have new array list implementations in your public API which don’t immediately compose with those others are using, like the std implementation. I’ve taken a similar approach in my project and cut down several std data structures to only the functionality I personally need - but these are internal to the library I’m working on. Given the importance of the std data structures as a common library and especially something as fundamental as ArrayList, it would be nice to see as little friction there as possible. There’s also the alternative of everyone deciding to use a particular dependency instead, like Abseil, Boost or Folly in the case of C++. But I think that was because of the failure of the C++ standard library. Zig seems much more like Golang and therefore the standard library is much more important. Though it’s still early days for the project of course and there are bigger priorities, so I’m not surprised it hasn’t been changed, especially given it’s used everywhere.

9

Are you changing the actual layout of the data? Otherwise I don’t see the point. code that is not used is not compiled, including std.

There are 8 append* functions, half of which are AssumeCapacity variants, so that leaves us 4 ‘core’ append functions: append, appendNTimes, appendSlice, appendUnalignedSlice. Reading the source for they all have different semantic implementations (directly setting value of a pointer, memset, memcopy). The only two that don’t differ that way are appendSlice and appendUnalignedSlice, which both use @memset. However the documentation for appendUnalignedSlice states that it should only be used if appendSlice would give a compile error.

I like the verbose API, exposing the core details to allow meaningful differences based on what the user actually needs rather than forcing us to go through a dispatch layer. Why should we all be forced to do that, especially since, as stated above, you don’t have to pay the cost of that extra code if you don’t use it.

1 Like
10

I’ve also made some other changes in my implementations, but that’s only part of the reason. You absolutely pay a cost in readability and maintainability of a codebase, even and especially for code that isn’t used. If the API instead had 1000 functions, I don’t think users would appreciate it.

1 Like
11

At least on current master there are 12. Just scrolling through that page might give an indication on what the cost of an unnecessarily large API surface is: It’s really tedious to find anything on that page. There is a total of 63 methods defined for ArrayList here. For comparison, rust’s Vec has 59 (not counting methods inherited from slices or trait methods), which is also a lot but and still Vec provides more actual functionality, despite many of these being simple conversion methods or mut/const variations. I am not suggesting here to replicate rust’s approach but the opposite, btw. Less is more.

12

You can’t actually do this. There are some slightly-magical rules which determine which functions in a container type can be dot-called using an instance of that type, but they don’t nest.

You could do this:

ArrayList.noalloc.appendSlice(&array, slice);

But not this:

array.noalloc.appendSlice(slice);

This has a meaning: self has an array of function pointers, indexed by the unsigned integer value NoAlloc, the pointer at that slot is then called with items.

The kind of change suggested in this brainstorm (leaving aside specific suggestions for how to do it) is almost certainly too fundamental to be considered at this point in the process.