Redis module header file does not translate successfully

[kristoff-it]

I want to use Zig to write a module for Redis but the automated translation of redismodule.h fails.

Ideally, I would like to be able to put in the same directory redismodule.h (https://github.com/antirez/redis/blob/unstable/src/redismodule.h) and this sample code (https://gist.github.com/kristoff-it/449b5e3a0283f6c9a4e443abe441482b), and be able to compile it.

In practice, that header file is doing some work for the user so there might be the possibility that it's not possible to fix everything.

First error:

This line and the next cause problems: https://github.com/antirez/redis/blob/unstable/src/redismodule.h#L362

C code:

void *getapifuncptr = ((void**)ctx)[0];
RedisModule_GetApi = (int (*)(const char *, void *)) (unsigned long)getapifuncptr;

Zig code:

var getapifuncptr: ?*c_void = @ptrCast([*c](?*c_void), ctx)[0];
RedisModule_GetApi = (?extern fn([*c]const u8, ?*c_void) c_int)(c_ulong(getapifuncptr));

Here's how I made it work:

 var getapifuncptr: ?*c_void = @ptrCast([*c](?*c_void), @alignCast(8, ctx))[0];
 RedisModule_GetApi = @ptrCast(?extern fn([*c]const u8, ?*c_void) c_int, getapifuncptr);

Second error:

(If I'm not mistaken, this is new in 0.4.0)
This line https://github.com/antirez/redis/blob/unstable/src/redismodule.h#L529

C code:

if (RedisModule_IsModuleNameBusy && RedisModule_IsModuleNameBusy(name)) return REDISMODULE_ERR;

Zig code:

if ((RedisModule_IsModuleNameBusy != 0) and (RedisModule_IsModuleNameBusy.?(name) != 0)) return 1;

I fixed it by calling @ptrToInt on RedisModule_IsModuleNameBusy

More general problem

If I understand correctly the addition of c_pointers is to make it less verbose to use automatically translated header files. In this case I'm still seeing a lot of optional types being generated.
I imagine this is caused by all the things that redismodule.h does, but unfortunately I don't know where the automated translation could be improved and where it is the headerfile that should try to do things differently.

Redis has an ABI stability promise towards modules, but if there is a change needed to make the translation work better that doesn't break it, I think there would be no problem in trying to fix these problems at the origin.

[stockholmux]

I'm also using zig to write a Redis module - I found another problem in the redismodule.h translation.

In redismodule.h file it has bit shifted #defines which don't seem to be working. Example:

#define REDISMODULE_READ (1<<0)

Doesn't seem to translate to anything in the redismodule.zig file, while #defines with straight numeric values are translated.

[daurnimator]

First error

I think this was fixed with #2318. Are you using 0.4.0 or master?

[andrewrk]

Hi @kristoff-it, thanks for filing this issue. I decided to prioritize this use case to unblock you.

The first thing I did is to go ahead and implement #1967, which is now done and landed in master branch. Now I'll go through and see what can be done about these other things.

[kristoff-it]

@daurnimator I was using Zig 0.4.0
@andrewrk thanks!

I've now cloned master and tried again. The second problem is now fixed (instead of == 0, Zig now produces == null), but the fist one still has an error:

/Users/loriscro/Documents/GitHub/zig-redismodule/transnew.zig:515:76: error: expected type 'c_ulong', found '?*c_void'
    RedisModule_GetApi = (?extern fn([*c]const u8, ?*c_void) c_int)(c_ulong(getapifuncptr));
                                                                           ^

[andrewrk]

These changes I think will be necessary to the example:

const redis = @cImport({
    @cInclude("./redismodule.h");
});

export fn HelloWorld_Command(ctx: ?*redis.RedisModuleCtx, argv: [*c]?*redis.RedisModuleString, argc: c_int) c_int {
    _ = redis.RedisModule_ReplyWithSimpleString.?(ctx, c"Hello World!");
    return redis.REDISMODULE_OK;
}

export fn RedisModule_OnLoad(ctx: *redis.RedisModuleCtx, argv: [*c]*redis.RedisModuleString, argc: c_int) c_int {
    if (redis.RedisModule_Init(ctx, c"testmodule", 1, redis.REDISMODULE_APIVER_1) == redis.REDISMODULE_ERR) {
        return redis.REDISMODULE_ERR;
    }

    if (redis.RedisModule_CreateCommand.?(ctx, c"test.hello", HelloWorld_Command, c"readonly", 0, 0, 0) == redis.REDISMODULE_ERR) {
        return redis.REDISMODULE_ERR;
    }

    return redis.REDISMODULE_OK;
}

Some notes about this:

• The c prefix on the strings will be able to go away after proposal: type for null terminated pointer #265 is implemented.
• If you're willing to make changes upstream, translate-c: ability to annotate types in C code for better translation #2457 will offer a way to use annotations to remove most if not all of the noisy ?s. I would welcome your ideas about how to encode such annotations in the C source.

Here's a smaller example of the c_ulong(getapifuncptr) thing:

void foo(void);
void bar(void) {
    void *func_ptr = foo;
    void (*typed_func_ptr)(void) = (void (*)(void)) (unsigned long) func_ptr;
}

incorrectly translates to the following zig code

pub extern fn foo() void;
pub export fn bar() void {
    var func_ptr: ?*c_void = @ptrCast(?*c_void, foo);
    var typed_func_ptr: ?extern fn() void = (?extern fn() void)(c_ulong(func_ptr));
}

I'm working on that one now.

[andrewrk]

After the above linked commit to master branch, the Zig code from my above comment now successfully builds with https://github.com/antirez/redis/blob/unstable/src/redismodule.h using zig build-obj test.zig -isystem . --library c

@stockholmux I'll see about getting support for the pattern #define REDISMODULE_READ (1<<0)

[andrewrk]

OK now the pattern #define foo (a << b) can be translated. Is there anything else to do for this use case, other than the issues mentioned in #2451 (comment)?

[kristoff-it]

Wow @andrewrk that's amazing work that you're doing.
We haven't yet tested the whole API, I'm sorry but it will take us more time to know if there's anything else, in the meantime I think you can consider this issue solved.

I too thought about the idea of adding some annotation to the C code, seems the only way to add reasonable ease of use without completely losing safety. I'll check out the issue and contribute if I anything useful to say.

As for adding them to the headerfile, I suspect the maintainers of the project would not be immediately open to adding extraneous annotations that don't belong to C (it's a bit more intrusive than just "rewording" the C code), but I can certainly make the case for it and, worst case, with that feature it would be easier for me to maintain a version of the headerfile for Zig, because my current zig-redismodule is definitely not "perfect software" :)

[andrewrk]

OK roger that @kristoff-it.

I'll close the issue for now; feel free to comment here if anything else comes up, and I'll re-open it.