ziglang / zig

General-purpose programming language and toolchain for maintaining robust, optimal, and reusable software.
https://ziglang.org
MIT License
34.36k stars 2.51k forks source link

`#define ZIG_TARGET_MAX_INT_ALIGNMENT` not being generated with `-femit-h` #16730

Open BeckmaR opened 1 year ago

BeckmaR commented 1 year ago

Zig Version

0.12.0-dev.17+49244dc0c

Steps to Reproduce and Observed Behavior

Base situation is the same as in #16676, but the new zig.h from 0.12.0 seems to work much better. However, there is one new error, which is that ZIG_TARGET_MAX_INT_ALIGNMENT is not being defined anywhere. Again, I built my library with zig build-lib app/src/lib.zig -target x86_64-freestanding -femit-h.

Expected Behavior

ZIG_TARGET_MAX_INT_ALIGNMENT probably should be defined in the generated lib.h. I found where it is generated in the code: https://github.com/ziglang/zig/blob/ac95cfe4496a5504b42bf37b02f34eff390fe956/src/link/C.zig#L233C31-L233C59 But that function is only called in one place, and the comment already says:

// This code path happens exclusively with -ofmt=c. The flush logic for
// emit-h is in `flushEmitH` below.

I assume that is already the error?

BeckmaR commented 1 year ago

On further investigation, the problem actually also persists when using -ofmt=c. The generated c-file contains the required #define, but the emitted header (non-surprisingly) still does not, but includes zig.h.

IMO, the generated code should not be platform-dependent like this. Also, the c-file does not include the generated header, but has its own definition of the function. This too is bad style IMO. The file should include its own header first, and only include other headers when required for internal functionality.