std.os.uefi: fix some mistakes

[wooster0]

This is an attempt to make std.os.uefi work with self-hosted (stage2); it is currently broken.

With this PR, the following now works:

const std = @import("std");

pub fn main() void {
    std.testing.refAllDeclsRecursive(std.os.uefi);
}

$ zig run x.zig --zig-lib-dir lib
$

However, it doesn't work with stage1:

$ zig run x.zig --zig-lib-dir lib -fstage1
./lib/std/os/uefi/protocols/simple_text_output_protocol.zig:7:38: error: struct 'std.os.uefi.protocols.simple_text_output_protocol.SimpleTextOutputProtocol' depends on itself
pub const SimpleTextOutputProtocol = extern struct {
                                     ^
./lib/std/os/uefi/tables/system_table.zig:27:5: note: while checking this field
    con_out: ?*SimpleTextOutputProtocol,
    ^
./lib/std/os/uefi/protocols/loaded_image_protocol.zig:10:33: error: struct 'std.os.uefi.protocols.loaded_image_protocol.LoadedImageProtocol' depends on itself
pub const LoadedImageProtocol = extern struct {
                                ^

This error only happens with the old compiler. I can't really figure out why. It only started happening after I added std.meta.FnPtr.
So I came here to get some feedback: is it fine to drop stage1 support for std.os.uefi? If so, we could also just use *const Fn directly instead of std.meta.FnPtr(Fn) where std.meta.FnPtr is supposed to serve for compatibility with both stage1 and stage2.

Also, the reason I changed all these packed structs to extern structs is because some of them were containing arrays and as we all know packed structs can no longer have them so I thought that maybe these should all be extern structs anyway so I just changed them all.
This might be wrong, which is why I'm asking for feedback or reviews.

CC @fifty-six

[fifty-six]

Yeah like leecannon mentioned, the old packed structs are equivalent to extern structs with all align(1). That's also the main reason I haven't gotten around to it, wanted to make some meta function to add the aligns instead of changing every struct because I am lazy

[wooster0]

Ah, okay! I see now. I figured that it would probably be more complicated. In that case I will exclude the bit size stuff for now so that we can add such a meta function and do the rest some other time.

[wooster0]

Now the issue with the stage1-only depends on itself error (see PR description) still stands though. Is it fine if we make the UEFI interface only available on stage2?

Well, I suppose we could conditionally load a different file in stage1.

But I tried to look ahead a little bit and there are really a lot of depends on itself errors in a bunch of files. Is this worth pursuing? Really seems like a bit of a nasty stage1 bug to workaround.

[fifty-six]

Alright I've written one now so I'll go ahead and PR it and then we can use that if it goes through.

[wooster0]

I totally missed that #12761 already does part of this apparently. My PR still contains some other UEFI-specific fixes though. I guess either will have to rebase.

[fifty-six]

Would you also be interested in me PRing your fork with the changes to the structs (maybe with a rebase to use CPacked) while we wait on that?

[wooster0]

Sure! I was kinda waiting for #12899 to be accepted/merged but I'm assuming it will probably be accepted in roughly that form, even if only temporarily until the language catches up and it will be possible to do that on a language level.

[fifty-six]

Ah given the issue with decls I might wait for something else to show up - was just considering working on my project so I'd need a local fork to get it building anyways and figured it'd be good to pr if I did so. Might just go with adding align(1) everywhere even though that's a pain.

[wooster0]

I don't think #6709 is going to happen anytime soon either (and I'm no longer convinced it should be allowed anyway).
Do we want to open a separate issue to bring more attention to this? I don't actually know of an issue that talks about this.
What should the syntax look like you think? extern struct align(1) {}?

[fifty-six]

Yeah, someone mentioned similar syntax in #10113 but I don't know of any issue for it.

[wooster0]

@MasterQ32

It's planned to have some kind of syntax like extern struct align(1) { … } which would be equivalent to the C version

Can you elaborate on this plan? Like who is planning this? Is anyone keeping track of this or already working on it?

[fifty-six]

Would we want to add align(1) to every field while we don't have language support for something similar? It's a bit of a pain but as-is uefi is largely just unusable under stage 2 due to the lack of it.

[wooster0]

I would prefer doing that in another PR, if at all. #13009 is tagged for 0.10 so it might get more priority soon, but I don't know.

So, I'm just waiting for this to be merged.

[fifty-six]

Ah ok that sounds ideal

[ominitay]

Would we want to add align(1) to every field while we don't have language support for something similar? It's a bit of a pain but as-is uefi is largely just unusable under stage 2 due to the lack of it.

Yes we do; as UEFI has Tier 2 support, we shouldn't be allowing it to regress.

[andrewrk]

Thanks!

[wooster0]

Yeah, otherwise, if it doesn't look like #13009 is going to happen anytime soon, we might as well add align(1) "manually" to every struct field with a regex or something. Then we can revert that commit after we have #13009.

But I think fixing the UEFI might have to wait until 0.10.1.