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

support more macros in translate-c #1085

Open
andrewrk opened this issue Jun 9, 2018 · 16 comments
Open

support more macros in translate-c #1085

andrewrk opened this issue Jun 9, 2018 · 16 comments
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. translate-c C to Zig source translation feature (@cImport)
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Jun 9, 2018

this is an issue to collect examples of macros that fail to translate, but there would be a very clear way to correctly translate the macro into a zig.

here's one for starters:

#define PPosition	(1L << 2) /* program specified position */
@andrewrk andrewrk added the enhancement Solving this issue will likely involve adding new logic or components to the codebase. label Jun 9, 2018
@andrewrk andrewrk added this to the 0.4.0 milestone Jun 9, 2018
@bheads
Copy link

bheads commented Jun 9, 2018

Macros as functions:

#define FOO__INIT (X)  (foo(X, X+X))

int foo(int x, int x2) {
        return x2 - x;
}

Currently only exports foo, would want FOO_INIT as a function too.

@andrewrk
Copy link
Member Author

andrewrk commented Jun 9, 2018

That one is actually not possible to translate perfectly. Consider:

int x = 0;
int y = FOO__INIT(++x);

In C the ++ happens 3 times, but in zig it would happen 1 time.

@bheads
Copy link

bheads commented Jun 9, 2018

Thats true that macros suck, but there is a lot of C code that makes macro functions, having a way to translate them, or maybe simpler would be to at least make them a comment so you can translate them by hand would help. One example I was dealing with was protobuffer, it generates a .c and .h file based on your .proto so tweaking it every time would be a pita.

@andrewrk
Copy link
Member Author

andrewrk commented Jun 9, 2018

make them a comment

That's a brilliant idea! Easy to do too.

@bheads
Copy link

bheads commented Jun 9, 2018

Just ran into this one from SDL2

/**
 *  \brief Used to indicate that you don't care what the window position is.
 */
#define SDL_WINDOWPOS_UNDEFINED_MASK    0x1FFF0000u
#define SDL_WINDOWPOS_UNDEFINED_DISPLAY(X)  (SDL_WINDOWPOS_UNDEFINED_MASK|(X))
#define SDL_WINDOWPOS_UNDEFINED         SDL_WINDOWPOS_UNDEFINED_DISPLAY(0)
#define SDL_WINDOWPOS_ISUNDEFINED(X)    \
            (((X)&0xFFFF0000) == SDL_WINDOWPOS_UNDEFINED_MASK)

/**
 *  \brief Used to indicate that the window position should be centered.
 */
#define SDL_WINDOWPOS_CENTERED_MASK    0x2FFF0000u
#define SDL_WINDOWPOS_CENTERED_DISPLAY(X)  (SDL_WINDOWPOS_CENTERED_MASK|(X))
#define SDL_WINDOWPOS_CENTERED         SDL_WINDOWPOS_CENTERED_DISPLAY(0)
#define SDL_WINDOWPOS_ISCENTERED(X)    \
            (((X)&0xFFFF0000) == SDL_WINDOWPOS_CENTERED_MASK)

@isaachier
Copy link
Contributor

I'm wondering about the current way macros are expanded. Basically, Zig tries to tokenize and parse the expression, putting the result into macro_table. The problem here, is that Zig isn't meant to be a full-blown C compiler, and I'm not really sure why we bother using the raw macros at all. Instead, can't we do the equivalent of clang -E <c-source> first, ignore the #defines in the output, and go straight to translating the result? It would make more sense to allow the clang lexer to handle the macros.

@0joshuaolson1
Copy link

Some macros are expected to be configured by command line parameters.

@ghost
Copy link

ghost commented Jun 10, 2018

re clang -E:

why we bother using the raw macros at all.

so Imagine you have

#define SDL_WINDOWPOS_CENTERED_MASK    0x2FFF0000u

and it gets inlined everywhere to 0x2FFF0000u

  • no one would know where this magic number came from
  • no one could change it efficiently/ in a safe way

now, this example is probably not very good because I guess zig can translate this to be a variable just fine but still.


make them a comment

I like that idea as well

@isaachier
Copy link
Contributor

isaachier commented Jun 10, 2018

I'm also fine with essentially commenting the source. This is more of an issue for more complex macros. For example, a for each loop macro would probably break. For a good idea of this, check out the array and object loop macros in Jansson.

Edit: Here is the Jansson example:

json_array_foreach(array, index, value) {
    /* block of code that uses index and value */
}

@mesbahamin
Copy link

Here's one I just ran into while trying to rewrite a debugger in Zig. It may be the same class of macro as the one in @bheads' comment.

#define	__WIFSTOPPED(status)	(((status) & 0xff) == 0x7f)
#define WIFSTOPPED(status)	__WIFSTOPPED (status)

These macros are found in my system's sys/wait.h file, and another header it #includes.

So being new to Zig, I was perplexed at first when I compiled this:

const c = @cImport({
    @cDefine("_NO_CRT_STDIO_INLINE", "1");
    @cInclude("sys/wait.h");
    // ...
});

export fn main(argc: c_int, argv: **u8) c_int {
    // ...
    while (c.WIFSTOPPED(wait_status)) {
        // ...
    }
    // ...
}

And got this error:

$ zig build-exe --library c main.zig 
~/code/dbg/main.zig:41:17: error: no member named 'WIFSTOPPED' in '(C import)'
        while (c.WIFSTOPPED(wait_status)) {
                ^

But sure enough:

$ zig translate-c /usr/include/sys/wait.h | grep WIFSTOPPED
$

It's easy enough to simply rewrite the necessary code in Zig and skip @cIncludeing the c header entirely, so that's probably what I'll do for now.

@andrewrk andrewrk added the contributor friendly This issue is limited in scope and/or knowledge of Zig internals. label Jan 6, 2019
@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Feb 10, 2019
@Akuli
Copy link
Contributor

Akuli commented Mar 15, 2019

ncurses has some funny macros that don't work, too

akuli@Akuli-Desktop:/usr/include$ grep A_STANDOUT curses.h 
#define WA_STANDOUT	A_STANDOUT
#define A_STANDOUT	NCURSES_BITS(1UL,8)
#define wstandout(win)      	(wattrset(win,A_STANDOUT))
akuli@Akuli-Desktop:/usr/include$ grep 'define NCURSES_BITS' curses.h
#define NCURSES_BITS(mask,shift) (NCURSES_CAST(chtype,(mask)) << ((shift) + NCURSES_ATTR_SHIFT))
akuli@Akuli-Desktop:/usr/include$ grep 'define NCURSES_CAST' curses.h
#define NCURSES_CAST(type,value) static_cast<type>(value)
#define NCURSES_CAST(type,value) (type)(value)
akuli@Akuli-Desktop:/usr/include$ grep 'define NCURSES_ATTR_SHIFT' curses.h
#define NCURSES_ATTR_SHIFT       8
akuli@Akuli-Desktop:/usr/include$ 

I ended up creating a C file just to figure out the resulting value of A_STANDOUT

#include <curses.h>
int main(void) { printf("%x\n", A_STANDOUT); }

Then I put it in my zig code

pub const A_STANDOUT: c_int = 0x10000;

Couldn't zig invoke the C preprocessor or something when it can't parse the macro? That wouldn't need to happen for most macros, so I don't think that would be noticably slower.

@andrewrk
Copy link
Member Author

andrewrk commented Mar 15, 2019

Couldn't zig invoke the C preprocessor or something when it can't parse the macro?

The C preprocessor is fully supported in terms of actual C top level declarations. If the .h file had used A_STANDOUT in a function prototype or definition it would have worked fine.

The problem in general here is that what the .h file is exporting with a #define FOO bar is, described in English, "The string 'FOO' in your code should expand to the string 'bar', whatever that ends up doing." That's a nonsensical expectation when you consider that C and Zig are entirely different languages, and in fact Zig intentionally does not have text substitution macros.

Consider, what should happen for this macro? #define LBRACE {

One thing that is possible is if the programmer informed Zig of the "type" of a macro. Then Zig could emit extra C code to make it work. Something like this:

const c = @cImport({
    @cMacroHint("A_STANDOUT", c_int);
    @cInclude("curses.h");
});

Then when Zig creates its C source text to hand off to clang, it would look like this:

#include "curses.h"
static const int MAGICPREFIX_A_STANDOUT = A_STANDOUT;

Then when translating symbols, Zig looks for the MAGICPREFIX and translates it as A_STANDOUT rather than its own attempt to parse the macro.

@daurnimator
Copy link
Contributor

lua has some quite simple macros that aren't translated, e.g.

LUA_API int   (lua_pcallk) (lua_State *L, int nargs, int nresults, int errfunc,
                            lua_KContext ctx, lua_KFunction k);
#define lua_pcall(L,n,r,f)      lua_pcallk(L, (n), (r), (f), 0, NULL)
#define lua_pushcfunction(L,f)  lua_pushcclosure(L, (f), 0)

@andrewrk andrewrk added the translate-c C to Zig source translation feature (@cImport) label Jun 16, 2019
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Aug 21, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Oct 23, 2019
@momumi
Copy link
Contributor

momumi commented Jan 1, 2020

#define MY_VEC  {0.0f, 0.0f, 0.0f}

#define MY_MAT  {{1.0f, 0.0f, 0.0f},                    \
                 {0.0f, 1.0f, 0.0f},                    \
                 {0.0f, 0.0f, 1.0f}}

can translate to

pub const MY_VEC = .{0.0, 0.0, 0.0,};

pub const MY_MAT = .{
    .{1.0, 0.0, 0.0,},
    .{0.0, 1.0, 0.0,},
    .{0.0, 0.0, 1.0,}
};

Related: need #3672 to allow casting these values to arrays.

@andrewrk andrewrk added the frontend Tokenization, parsing, AstGen, Sema, and Liveness. label Jan 2, 2020
@via
Copy link
Contributor

via commented Jan 4, 2020

translate-c doesn't appear to handle a sizeof() in a macro:

#define USB_DT_DEVICE_SIZE sizeof(struct usb_device_descriptor)
/home/via/dev/viaems-test-bench/zig-cache/o/tWu0K_n2cLYzck_aU00RC2xX0srpC21S8nDbd87zMrdUBlHo3Awr88YyR192ASkm/cimport.zig:369:32: error: unable to translate C expr: expected ',' or ')'
pub const USB_DT_DEVICE_SIZE = @compileError("unable to translate C expr: expected ',' or ')'");
                               ^

@andrewrk
Copy link
Member Author

andrewrk commented Jan 7, 2020

Alright enough of these are solved that it's time to start trying to close this issue in favor of individual issues. Please, from now on, new issues for each different macro, and let's make sure the examples in this issue have test coverage and then it can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. translate-c C to Zig source translation feature (@cImport)
Projects
None yet
Development

No branches or pull requests

9 participants