Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Syntax for array default initialization #4847

Closed
Srekel opened this issue Mar 29, 2020 · 7 comments
Closed

Proposal: Syntax for array default initialization #4847

Srekel opened this issue Mar 29, 2020 · 7 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@Srekel
Copy link

Srekel commented Mar 29, 2020

So I wanted to create an static array holding structs that has default values. I looked through the docs and tried numerous things before I figured out how to do this. (Or rather, I got help on Discord)

const TestData = struct {
    a: i32 = 1,
    b: i32 = 2,
};

Best solution I've found:

var list = [_]TestData {.{}} ** 128;

I think this however would be more intuitive:

var list: [128]TestData = .{};

And/Or this:

var list = [128]TestData{};

Sure, it's another way to do the same thing, which is generally not zig-like, but it's not really "new" syntax, just uses already existing ones.

It's cleaner and is something I would have arrived at "naturally" pretty much without having to look at the documentation.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Mar 29, 2020
@andrewrk andrewrk added this to the 0.7.0 milestone Mar 29, 2020
@Tetralux
Copy link
Contributor

Tetralux commented Mar 29, 2020

I'd suggest:

var list: [128]TestData;

to be functionally equivalent to this:

var list: [128]TestData = undefined;
var i: usize = 0;
while (i < list.len) : (i += 1) {
    list[i] = {};
}

For primitive types, like var buf: [4096]u8; it could just be a memset to zero.

Note that on master, = {} doesn't actually work like it used to, however, for clarity, it used to initialize every field of the struct to its default value based on type, and default struct field values in the struct definition.

When I first tried Zig, I was curious why this syntax was not allowed; I think it bares repeating here.

@Tetralux
Copy link
Contributor

Tetralux commented Mar 29, 2020

After discussions on Discord, I'll try to summarise here.

The consenus seemed like library functions would do well for this.

// all fields are their zero-value
var list = mem.zeroes([128]TestData);

// initialize fields to default values declared in the struct;
// recurses fields that are structs; compile error if not all fields have defaults
var list = mem.defaults([128]TestData);

// fields with defaults are set, rest are zeroed, recurses fields that are structs
var list = mem.defaultsOrZeroes([128]TestData);

// same as defaultsOrZeroed, except undefined instead
var list = mem.defaultsOrUndefined([128]TestData);

This is because of wanting to keep explicit initialization, and also how var s = S{} has a contract with the struct definition that all it's fields without default values were specified. (In that example, all fields have defaults.)

It's also because hidden code generation (i.e: any kind of initialization code being run should be explicit).

This would make the buffer example fairly simple:

var buf = mem.zeroes([4096]u8);

A bit more long winded than ideal, but it does at least address the concern that I have with that example, and it's honestly not that bad, considering the wish to be explicit.

But yes, the defaultsOrZeroes/defaults would suit the OP's example, AFAICT.

@Sobeston
Copy link
Sponsor Contributor

I like the idea of just using std.mem.zeroes.

Only issue with it:
var foo = std.mem.zeroes([500]u8);
It seems an array of size 500 elements or more will cause error: evaluation exceeded 1000 backwards branches.

Rewriting std.mem.zeroes in how it handles arrays may sidestep this problem.

Sobeston added a commit to Sobeston/zig that referenced this issue Mar 30, 2020
Before (when given an array with many elements):
```
zig\std\mem.zig:345:13: error: evaluation exceeded 1000
backwards branches
            for (array) |*element| {
            ^
```
related to ziglang#4847 (comment)
@Tetralux
Copy link
Contributor

Tetralux commented Mar 30, 2020

@Sobeston Hmmm. That's unfortunate.
I'm not sure that makes too much sense... 500 iterations; 1000 backwards branches?
I don't even know what means!

Regardless though, I'm not sure why you shouldn't be able to loop 500 times. 🤔

@Sobeston
Copy link
Sponsor Contributor

@Tetralux there's currently a loop in std.mem.zeroes for arrays for each element. Comptime is deliberately limited when it comes to looping (you can think of the end of a loop going back to the start as a branch going backwards). My PR is a fix for this - no looping, just does it in one step.

@Tetralux
Copy link
Contributor

Tetralux commented Mar 30, 2020

I feel like that error message is needlessly esoteric. I guess it's a generic message. Definitely confusing though; "I didn't write a backwards branch - I wrote a loop."

To make that worse, I don't really see why it would have 1000 branches while looping 500 times; where's that factor of 2 coming from?

@Sobeston
Copy link
Sponsor Contributor

To make that worse, I don't really see why it would have 1000 branches while looping 500 times; where's that factor of 2 coming from?

(inside std.mem.zeroes)

        .Array => |info| {
            var array: T = undefined;
            for (array) |*element| {
                element.* = zeroes(info.child);
            }
            return array;
        },

Each iteration calls zeroes while already in zeroes, meaning another backwards branch. So there's your factor of 2.

andrewrk pushed a commit that referenced this issue Mar 30, 2020
Before (when given an array with many elements):
```
zig\std\mem.zig:345:13: error: evaluation exceeded 1000
backwards branches
            for (array) |*element| {
            ^
```
related to #4847 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

4 participants