Skip to content

Conversation

@Vexu
Copy link
Member

@Vexu Vexu commented Jul 20, 2022

This makes Aro pass all tests with #11798 worked around and #11867 applied.

@andrewrk
Copy link
Member

I think we can merge this since it is an improvement over status quo, which produces an invalid LLVM module, although I would like to improve the test case to actually check that the code is correctly compiled rather than only checking that it does not crash the compiler.

Note however that we do have some more work to do with pointers to packed structs:

const Symbol = struct {
    index: u16,
};

const Relocation = packed struct {
    symbol: *Symbol,
    a: u32,
};

var x: Relocation = .{
    .symbol = &s,
    .a = 1234,
};

var s: Symbol = undefined;

test "global packed struct with pointer field" {
    _ = x;
}
Invalid bitcast
i64 bitcast (%test3.Symbol* @test3.s to i64)

thread 81531 panic: LLVM module verification failed

In some cases it is impossible to lower code like this because there is no such relocation supported by the object file. However, I think we can allow it when it does work, as long as we can detect when it won't work and emit a compile error.

Now it checks that the code is correctly compiled rather than only checking
that it does not crash the compiler.
@andrewrk andrewrk merged commit c1f3aca into ziglang:master Jul 20, 2022
@Vexu Vexu deleted the packed-ptr branch July 20, 2022 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants