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

Assign an array to sentinel array #16181

Open
Araminosian opened this issue Jun 24, 2023 · 6 comments
Open

Assign an array to sentinel array #16181

Araminosian opened this issue Jun 24, 2023 · 6 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@Araminosian
Copy link

Araminosian commented Jun 24, 2023

Zig Version

0.10.1

Steps to Reproduce and Observed Behavior

var array: [2]u8 = .{ 1, 2 };
var sentinel_array: [2:0]u8 = array;

error: expected type '[2:0]u8', found '[2]u8'

Expected Behavior

There is no reason to prohibit this operation. That is just a copy.

@Araminosian Araminosian added the bug Observed behavior contradicts documented or intended behavior label Jun 24, 2023
@rohlem
Copy link
Contributor

rohlem commented Jun 24, 2023

I agree. If the implicit conversion is somehow deemed controversial (not sure why), at least allowing it explicitly via @as would be nice - in status-quo you need a utility function to implement it.
EDIT: Actually, I'm not a fan of @as allowing a coercion that isn't allowed implicitly.
(IIRC in status-quo, the only difference is that the length of an implicit coercion sequence is limited, which can be extended by @as.)

Another alternative would be a dedicated @sentinelCast (with deduced result type) for maximum explicitness.

@andrewrk andrewrk added proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. and removed bug Observed behavior contradicts documented or intended behavior labels Jun 24, 2023
@andrewrk andrewrk added this to the 0.12.0 milestone Jun 24, 2023
@matu3ba
Copy link
Contributor

matu3ba commented Jun 25, 2023

There is no reason to prohibit this operation. That is just a copy.

Consider

var array: [100_000]u8 = .{ 1, 2 };
var sentinel_array: [100_000:0]u8 = array;
  1. It's not evident from = array, that several copies, most likely a loop is used in the assembly with potential significant time for execution.
  2. I dont see the significant advantage over @memcpy
  3. Having multiple occurences of
var array: [2]u8 = .{ 1, 2 };
var sentinel_array: [2:0]u8 = array;

which are not as helper functions smells like a bad design. Or do you have concrete code examples, where an inline function with comptime to create the zero-terminated memory structure[s] of a given array size is not possible?

@InKryption
Copy link
Contributor

It's not evident from = array, that several copies, most likely a loop is used in the assembly with potential significant time for execution.

This is already the case for non-sentinel copies of arrays. This would be an argument against status quo as well.

@matu3ba
Copy link
Contributor

matu3ba commented Jun 25, 2023

This is already the case for non-sentinel copies of arrays. This would be an argument against status quo as well.

For comptime-assignments, yes. For runtime-assignments, I think this is not correct.
Assigning slices certainly works, because its a pointer + len to the bytes, but not arrays themself.

Or do you have a brief counter-example?

Not a fan, but consistent.

Reason why I dislike the current behavior:

test {
    var x: u8 = 100;
    var a: [1_000]u8 = .{x} ** 1_000;
    var b: [1_000]u8 = undefined;

    // a pile of code

    b = a; // spot the performance issue

    // a pile of code
}

@mlugg
Copy link
Member

mlugg commented Jun 25, 2023

For comptime-assignments, yes. For runtime-assignments, I think this is not correct. Assigning slices certainly works, because its a pointer + len to the bytes, but not arrays themself.

Or do you have a brief counter-example?

I mean... just try it? You can totally copy arrays, they're nothing special:

test {
    var x: u8 = 100;
    var a: [1000]u8 = .{x} ** 1000;
    var b: [1000]u8 = a;
    _ = b;
}
All 1 tests passed.

@kfird214
Copy link

kfird214 commented Jul 17, 2023

I think array copies should be explicit anyway.
@mlugg example should not work

test {
    var x: u8 = 100;
    var a: [1000]u8 = .{x} ** 1000;
    var b: [1000]u8 = a;
    _ = b;
}

code on compiler explorer
The compiler will use memset under the hood.
I would consider that a hidden control flow

fun:
  push rax
  mov edi, offset example.a
  mov edx, 1000
  mov esi, 100
  call memset@PLT
  mov edi, offset example.b
  mov edx, 1000
  mov esi, 100
  call memset@PLT
  mov edi, offset example.a
  mov esi, offset example.b
  pop rax
  jmp f@PLT

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

7 participants