zig icon indicating copy to clipboard operation
zig copied to clipboard

missing constCast in c fn call cause compiler error without any tips and notes

Open oxfrancis opened this issue 9 months ago • 11 comments

Zig Version

0.12.0

Steps to Reproduce and Observed Behavior

after the recent feature in 0.12, never mutated local variable will cause a compiler error "local variable is never mutated", with note "consider using const". but if it happens in a c function call where missing a constCast, the compiler gives no note, just fails, make it hard to locate the causing.

Expected Behavior

just gives some notes on error

oxfrancis avatar Apr 28 '24 01:04 oxfrancis

Could you provide a code sample that reproduces this issue?

silversquirl avatar Apr 28 '24 01:04 silversquirl

Could you provide a code sample that reproduces this issue?

Could you provide a code sample that reproduces this issue?

c.XSetIMValues(null, c.XNDestroyCallBack, &c.XIMCallback{.client_data=null, .callback=null}, c.NULL) in 0.11, this will be ok, but it will cause the compiler fails in 0.12.0 without any notes.

the workaround is: c.XSetIMValues(null, c.XNDestroyCallBack, @constCast(&c.XIMCallback{.client_data=null, .callback=null}), c.NULL)

oxfrancis avatar Apr 28 '24 02:04 oxfrancis

There's still not enough context here to help diagnose what is going on - do you have a link to a repo we could look at? At first glance it looks like you're taking the address of a temporary variable (via &c.XIMCallback{.client_data=null, .callback=null}) which has produced a const pointer since 0.10.0 - so using @constCast is the wrong solution - you should either update the prototype to accept a const pointer to XIMCallback, or you should create var callback: c. XIMCallback = .{ .client_data = null, .callback = null } and pass &callback to c.XSetIMValues

ehaas avatar Apr 28 '24 04:04 ehaas

@ehaas @silversquirl here is the problem:

_ = std.c.printf("format", .{"abc"}); or _ = std.c.printf("format", &.{"abc"}); this simple code will make compiler fails without any hints.

_ = std.c.printf("format", @constCast(&.{"abc"})); // this is ok

unisgn avatar May 01 '24 19:05 unisgn

Stack Trace
/home/dr/zig/lib/std/debug.zig:403:14: 0x8004b53 in assert ()
    if (!ok) unreachable; // assertion failure
             ^
/home/dr/zig/src/Sema.zig:1904:11: 0x8ed6e2f in resolveType ()
    assert(air_inst != .var_args_param_type);
          ^
/home/dr/zig/src/Sema.zig:4648:32: 0x94b33e9 in zirValidateArrayInitTy ()
    const ty = sema.resolveType(block, ty_src, extra.ty) catch |err| switch (err) {
                               ^
/home/dr/zig/src/Sema.zig:1377:48: 0x8dfaf1c in analyzeBodyInner ()
                try sema.zirValidateArrayInitTy(block, inst, true);
                                               ^
/home/dr/zig/src/Sema.zig:910:30: 0x891bded in analyzeInlineBody ()
    if (sema.analyzeBodyInner(block, body)) |_| {
                             ^
/home/dr/zig/src/Sema.zig:936:39: 0x8510abd in resolveInlineBody ()
    return (try sema.analyzeInlineBody(block, body, break_target)) orelse .unreachable_value;
                                      ^
/home/dr/zig/src/Sema.zig:7414:65: 0x9e52319 in analyzeArg ()
                const uncoerced_arg = try sema.resolveInlineBody(block, arg_body, zir_call.call_inst);
                                                                ^
/home/dr/zig/src/Sema.zig:7977:49: 0x96c351d in analyzeCall ()
            arg_out.* = try args_info.analyzeArg(sema, block, arg_idx, param_ty, func_ty_info, func);
                                                ^
/home/dr/zig/src/Sema.zig:7125:43: 0x9387c04 in zirCall__anon_89488 ()
    const call_inst = try sema.analyzeCall(block, func, func_ty, callee_src, call_src, modifier, ensure_result_used, args_info, call_dbg_node, .call);
                                          ^
/home/dr/zig/src/Sema.zig:1015:62: 0x8dee6e2 in analyzeBodyInner ()
            .field_call                   => try sema.zirCall(block, inst, .field),

My personal opinion is that array init without an explicit type should be illegal in varargs.

Rexicon226 avatar May 01 '24 20:05 Rexicon226

@ehaas @silversquirl here is the problem:

_ = std.c.printf("format", .{"abc"}); or _ = std.c.printf("format", &.{"abc"}); this simple code will make compiler fails without any hints.

_ = std.c.printf("format", @constcast(&.{"abc"})); // this is ok

the problem seems to be that you can't use a temporary variable in a va_list, unless it's a @constCast modified pointer

unisgn avatar May 01 '24 21:05 unisgn

The examples above trigger an assertion in handling validate_array_init_[ref_]ty due to missing handling for var_args_param_type. That should be simple to fix. That said: @andrewrk, is there any reason var_args_param_type exists at all as opposed to simply using generic_poison_type? I suppose it could help us emit errors for type-unsafe behavior on API boundaries, such as this exact one.

@unisgn, the issue is not directly related to @constCast -- this code is invalid. You can't pass a Zig tuple to a C function and expect it to work. C varargs are, well, varargs -- use printf as you would in C.

mlugg avatar May 01 '24 21:05 mlugg

The examples above trigger an assertion in handling validate_array_init_[ref_]ty due to missing handling for var_args_param_type. That should be simple to fix. That said: @andrewrk, is there any reason var_args_param_type exists at all as opposed to simply using generic_poison_type? I suppose it could help us emit errors for type-unsafe behavior on API boundaries, such as this exact one.

@unisgn, the issue is not directly related to @constCast -- this code is invalid. You can't pass a Zig tuple to a C function and expect it to work. C varargs are, well, varargs -- use printf as you would in C.

let me show some more case.

const S = struct {a: u8=0}; var var_s = S{}; const const_s = S{};

_ = std.c.printf("",

var_s, // fail#1 with some tips S{}, // also fail#2 with some tips

&var_s, // ok#1 &const_s, // ok#2

&S{}, // fail#3 without any tips @constCast(&S{}), // ok#3

.{"str"}, // fail#4 without any tips &.{"str"}, // fail#5 without any tips @constCast(&.{"str"}), // ok#4 );

here is my question: 1, why fail#4 gives no tips like fail#1 and fail#2 2, since ok#2 is ok, why fail#3 fails, and give no tips. 3, since fail#3 fails, why ok#3 and ok#4 is ok

note 1, think tuple as an anonymous struct 2. the printf is chosen as a convenient fn that takes a varargs to test.

unisgn avatar May 01 '24 23:05 unisgn

  1. bug (the one I described above)
  2. bug (cc anyone working on this: coerce_ptr_elem_ty also needs a fix)
  3. it's a bug that fail#3 fails

Varargs aren't as tested as they should be! I might quickly put a PR up to fix this in a bit.

mlugg avatar May 02 '24 00:05 mlugg

Great job. But i have no idea how to cc about the coerce_ptr_elem_ty​ you said

---- Replied Message ---- | From | Matthew @.> | | Date | 05/02/2024 08:30 | | To | ziglang/zig @.> | | Cc | oxfrancis @.>, Author @.> | | Subject | Re: [ziglang/zig] missing constCast in c fn call cause compiler error without any tips and notes (Issue #19781) |

bug (the one I described above) bug (cc anyone working on this: coerce_ptr_elem_ty also needs a fix) it's a bug that fail#3 fails

Varargs aren't as tested as they should be! I might quickly put a PR up to fix this in a bit.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>

oxfrancis avatar May 02 '24 03:05 oxfrancis

Sorry, I only write "cc" to indicate a note in my comment to another party (in this case to whoever ends up working on the fix). It's not relevant to you, and since I've fixed the issue in #19835 (currently failing CI but it is a fix), it didn't end up being relevant to anyone else either!

mlugg avatar May 02 '24 04:05 mlugg