With some meta programming, you can also do something like:

const kind = a.smith.enumWeighted(Kind, .{
 .underscore = 6, 
 .regular_identifier = 3, 
 .quoted_identifier = 1, 
 .copy_identifier = 6, 
})

or maybe even

const kind = a.smith.autoEnumWeighted(.{
 .underscore = 6, 
 .regular_identifier = 3, 
 .quoted_identifier = 1, 
 .copy_identifier = 6, 
})

This is slightly more concise syntactically, and comptime-checks that enum is synced with weights.

We use this pattern fairly frequently at TigerBeetle.

In this scenario such function would not be applicable as copy_identifier is being dynamically included in the weights (via slicing them). Adding such a function also impedes defining weights via ranges which is more efficient (and concise); currently there is nowhere else in AstSmith which this would be applicable. So, I do not believe such a function is yet justified.

I think it's still works with dynamic weights, and it seems that

const kind = a.smith.autoEnumWeighted(.{
 .underscore = 6, 
 .regular_identifier = 3, 
 .quoted_identifier = 1, 
 .copy_identifier = if (a.prev_ids_len == 0) 0 else 6, 
})

is a touch more direct way to express optionality than weight slicing. I do like the weights construct though, its' very general!

Actually, isn't the code buggy at present?

 const kind_weights: [4]Weight = .{
 .value(Kind, .underscore, 6),
 .value(Kind, .regular_identifier, 3),
 .value(Kind, .quoted_identifier, 1),
 .value(Kind, .copy_identifier, 6),
 };
 const n_weights = @as(usize, 2) + @intFromBool(a.prev_ids_len != 0);

It should be @as(usize, 3), not 2, right?

(NB: here and elsewhere, I intend only to point out what's possible, you of course have more context what works best in context of smith.zig!)

Good catch!

Regarding that approach for dynamic weights, it would require extra logic in enumWeighted since weights of zero are disallowed to simplify checking if a value is in a set of weights. This is also another reason to keep the logic in the callee who can then construct the weights more efficiently.

For TigerBeetle, we ended up introducing a Ratio type to specify the weights, so the equivalent call would be a.smith.boolWeighted(ratio(1, 8)). The reason for that is that it makes easier to make Ratio a parameter:

a.smith.boolWeighted(a.options.ptrCastChance);

And that is useful because it enables swarm testing --- you can generate ptrCastChance itself from fuzzer input once at the start of the test. So, instead of all examples having the same average ratio of pointer casts, you can get examples which are mostly pointer casts, and examples which are hardly ever pointer casts. And SWARM testing literature (and common sense) tells that the latter is potentially more interesting.

Interesting idea. Note that the current fuzzer implementation does not effectively support swarm testing as it reuses previous values independent of their current weights (as long as it is still valid). Also, the use for this Ratio type is already fulfilled by storing the ratio with []const Weight and using valueWeighted, which also avoids having to reconstruct the weights each time. And as such, boolWeighted(a, b) can be viewed as a shorthand for valueWeighted(bool, ratio(a, b)).

Yeah, selecting a random item of a list is a fairly frequent use-case. For TB, we have an API that gives you a random index:

ids[a.smith.index(ids)];

This seems like a sweat spot --- the API that gives you item needs to be polymorphic with respect to const, and doesn't work for parallel array. But the API that gives you just the index works for all these cases and is fairly convenient.

I don't see any problem with such a function (except it should take ids.len instead to avoid being a generic function) and it does come up quite a few times in this. I will add this if I need to push the branch since it is not strictly necessary.

Lately I've come to the conclusion that pretty much any buffer in a struct which has more than one reasonable value should be a slice, with externally provided memory (rather than an array, embedded within the struct).

I won't make this block the PR from landing, but I wanted to offer you this consideration and see what you think about it.

Was this intentional?

before:

0.16.0-dev.3061+9b1eaad13
> zig run new_while.zig
new_while.zig:4:20: error: expected ':' before while continue expression
 while (i != 0) (i -= 1) {std.debug.print("i = {}", .{i});}
 ^

after:

0.16.0-dev.3100+ce3f25452
> zig run new_while.zig
new_while.zig:4:23: error: expected ')', found '-='
 while (i != 0) (i -= 1) {std.debug.print("i = {}", .{i});}
 ^~

Yes. It's to allow while (i != 0) (expr) without a newline between the parenthesis, which is consistent with the PEG. I do agree the new error is a bit worse but the old behavior was incorrect as it was.