Skip to content

#define BIT(n) Shift Operand (<<) that's translated from C is not working as expected #9276

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

Open
ShoshinX opened this issue Jul 1, 2021 · 2 comments
Labels
bug Observed behavior contradicts documented or intended behavior translate-c C to Zig source translation feature (@cImport)
Milestone

Comments

@ShoshinX
Copy link

ShoshinX commented Jul 1, 2021

Hi,

I was working on importing a set of C libraries into zig to be used in zig. However, I found a bug either in the translate-c code or in the zig compiler. The following is the smallest example I found as I was exploring the compilation errors.

This is using Zig 0.8.0 release.

I was exporting a C preprocessor BIT(n) defined as follows:

#define BIT(n) (1ul << n)

into zig. Zig translated it into

pub inline fn BIT(n:anytype) @TypeOf(@as(c_ulong,1) << n) {
  return 1 << n;
}

When I tried using it through const c = @cInclude(...) or the translated cimport.zig
I get an error as the following

error: expected type 'u6', found 'c_ulong'
fn bit(n: anytype) @TypeOf(@as(c_ulong, 1) << n) {
                                              ^
...
note: unsigned 6-bit int cannot represent all possible unsigned 64-bit values
fn bit(n: anytype) @TypeOf(@as(c_ulong, 1) << n) {
                                              ^

I found out from Spex_guy#0444 that this could be a bug in the zig translate-c.
Here's the following additional info Spex_guy provided me

"Zig has extra rules about shift operands, which we are planning to change to be less restrictive. The current rule is that the rhs of a shift must be an unsigned int type with ceil(log2(number of bits in lhs type)) bits. If you don't know the lhs type statically, you can use %%std.math.Log2Int to calculate the rhs type at comptime.

translate-c should probably translate shifts to have the same semantics as C, and add an implicit cast of the rhs, I think this does count as a C translation bug."

@ShoshinX
Copy link
Author

ShoshinX commented Jul 1, 2021

Further investigations, I found out that adding comptime before the parameter n removes the error, so pub inline fn BIT(comptime n:anytype) @TypeOf(@as(c_ulong,1) << n) now works. However, I think this is just a bandaid fix for now i.e I will have to rewrite the #define BIT(n)in zig and somehow create a function that handles it during runtime instead of the current comptime bandaid fix

@Vexu Vexu added bug Observed behavior contradicts documented or intended behavior translate-c C to Zig source translation feature (@cImport) labels Jul 1, 2021
@Vexu Vexu added this to the 0.10.0 milestone Jul 1, 2021
@AssortedFantasy
Copy link

AssortedFantasy commented Jul 14, 2021

This bug is because zig requires an awkward, and in my opinion unessasary cast because it doesn't implicitly @intCast to the correct type.

There are proposals related to this. See my comment in #7605 for this.

The TL;DR is that a << b should have semantics of a << @intCast(@import("std").math.Log2Int(a), b).

Anyway doing this also fixes the macro.

Edit: lmao didn't read, Already mentioned in issue by Spex.

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

No branches or pull requests

3 participants