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

translate-c: negative array indices don't work #8556

Closed
ehaas opened this issue Apr 17, 2021 · 6 comments · Fixed by #8589
Closed

translate-c: negative array indices don't work #8556

ehaas opened this issue Apr 17, 2021 · 6 comments · Fixed by #8589
Labels
bug Observed behavior contradicts documented or intended behavior translate-c C to Zig source translation feature (@cImport)
Milestone

Comments

@ehaas
Copy link
Sponsor Contributor

ehaas commented Apr 17, 2021

In C, array indexing is just shorthand for pointer arithmetic, but in Zig array subscripts are required to be unsigned. That means any C code which uses a negative array index (uncommon, but legal) can't be translated directly to a Zig array indexing operation.

From the C standard

The definition of the subscript operator [] is that E1[E2] is identical to (*((E1)+(E2))).

The if statement in this valid C code

#include <stdlib.h>
int main(void) {
	int foo[] = {0,1,2,3,4};
	int *bar = foo + 2;
	if (bar[-1] != 1) abort();
	return 0;
}

produces this line when translated (error: attempt to cast negative value to unsigned integer):

if (bar[@intCast(c_uint, -@as(c_int, 1))] != @as(c_int, 1)) {

One solution would be to always translate C array index operations to Zig pointer arithmetic:

if ((bar + @bitCast(usize, @as(isize, -@as(c_int, 1)))).* != @as(c_int, 1)) {

or bitcast indices to usize:

if (bar[@bitCast(usize, @as(isize, -@as(c_int, 1)))] != @as(c_int, 1)) {

Which is pretty ugly but I think guaranteed to work.

Happy to implement either solution (or something better if there are other ideas) if people think it's worthwhile.

@daurnimator
Copy link
Collaborator

See #1738

@folkertdev
Copy link

I'm running into a variation of this issue. This time using

> zigm version
0.8.0-dev.1981+fbda9991f

This C code:

    static const int bid_breakpoints_binary32[] = { 1,2,3,4 };

    int main() { 
        int index = -80;
        int m_min = (bid_breakpoints_binary32 + 80)[index];

        return 0;
    }

Generates this zig code

    pub const bid_breakpoints_binary32: [4]c_int = [4]c_int{
        1,
        2,
        3,
        4,
    };
    pub export fn main() c_int {
        var index: c_int = -@as(c_int, 80);
        var m_min: c_int = @ptrCast([*c]const c_int, @alignCast(@alignOf(c_int), &bid_breakpoints_binary32)) + @bitCast(usize, @intCast(isize, @as(c_int, 80)))[@intCast(c_uint, index)];
        return 0;
    }

Which gives this error

./generated.zig:10:156: error: array access of non-array type 'usize'
var m_min: c_int = @ptrCast([*c]const c_int, @alignCast(@alignOf(c_int), &bid_breakpoints_binary32)) + @bitCast(usize, @intCast(isize, @as(c_int, 80)))[@intCast(c_uint, index)];

Part of the problem I think is that a surrounding pair of parentheses is dropped. Additionally the index is negative, which will later fail with the message in the original post:

panic: attempt to cast negative value to unsigned integer

@Vexu Vexu added bug Observed behavior contradicts documented or intended behavior translate-c C to Zig source translation feature (@cImport) labels Apr 19, 2021
@Vexu Vexu added this to the 0.9.0 milestone Apr 19, 2021
@RogierBrussee
Copy link

RogierBrussee commented Apr 19, 2021

How about

if (bar[-%@as(usize, 1)] != @as(c_int, 1)) {

or (because unary -% currently seems not to work (see #8574, also #1770, #7951))

if (bar[@as(usize, 0) -% 1] != @as(c_int, 1)) {

Arithmetic with unsigned integers in C is modular arithmetic so all arithmetic operators should be translated as +%, -% *% anyway.

@ehaas
Copy link
Sponsor Contributor Author

ehaas commented Apr 19, 2021

That would work if it's known at compile time but it won't work for runtime negative integers.

@RogierBrussee
Copy link

RogierBrussee commented Apr 20, 2021

I think for signed integer variables,

var index : c_int = -1000;
const b = bar [@intcast(usize, @as(isize, index))];

should work because in C (assuming sizeof(void*) == sizeof(long)) if given

struct foo* bar;
int index;

then

&bar[ index] == bar + index
== bar + (signed long)index
== (typeof(&bar[0]))((unsigned long)bar + sizeof(bar[0])*(unsigned long)(signed long)index)
== bar + (unsigned long)(signed long)index

where in the third line all arithmetic is modular (aka wrapping).

IMHO this would be more natural if Zig had modular integers (see #7512) because that is what really going on here.

@kyle-github
Copy link

kyle-github commented Apr 20, 2021

assuming sizeof(void*) == sizeof(long)

Uh... At least with Windows (P64, not LP64), long is 32-bits on 64-bit systems, IIRC.

(Edit) I think you need to use uintptr_t for unsigned long and intptr_t for long.

ehaas added a commit to ehaas/zig that referenced this issue Apr 21, 2021
Take advantage of wrapping pointer arithmetic to enable negative array
subscripts.

Fixes ziglang#8556
ehaas added a commit to ehaas/zig that referenced this issue Apr 27, 2021
A rather complicated workaround for handling signed array subscripts.
Once `[*]T + isize` is allowed, this can be removed.

Fixes ziglang#8556
ehaas added a commit to ehaas/zig that referenced this issue May 7, 2021
A rather complicated workaround for handling signed array subscripts.
Once `[*]T + isize` is allowed, this can be removed.

Fixes ziglang#8556
ehaas added a commit to ehaas/zig that referenced this issue May 10, 2021
A rather complicated workaround for handling signed array subscripts.
Once `[*]T + isize` is allowed, this can be removed.

Fixes ziglang#8556
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 May 19, 2021
andrewrk pushed a commit to ehaas/zig that referenced this issue Jul 28, 2021
A rather complicated workaround for handling signed array subscripts.
Once `[*]T + isize` is allowed, this can be removed.

Fixes ziglang#8556
andrewrk pushed a commit that referenced this issue Jul 28, 2021
A rather complicated workaround for handling signed array subscripts.
Once `[*]T + isize` is allowed, this can be removed.

Fixes #8556
@andrewrk andrewrk modified the milestones: 0.10.0, 0.9.0 Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior translate-c C to Zig source translation feature (@cImport)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants