Declare structs as anonymous within anonymous unions

[ZehMatt]

Declaring named types within anonymous unions is not allowed, GCC fails to build the code otherwise.

[athre0z]

Under which conditions does this fail to build? It's compiling perfectly fine with gcc both on my local box and the CI. Removing the names will break the code-generation of various bindings -- that's why the structs are named in the first place.

[ZehMatt]

Under which conditions does this fail to build? It's compiling perfectly fine with gcc both on my local box and the CI. Removing the names will break the code-generation of various bindings -- that's why the structs are named in the first place.

Are you building it as C11? The issue is including it from a cpp compilation unit. You'll run into
error: ‘struct ZydisDecodedOperand_::<unnamed union>::ZydisDecodedOperandReg_’ invalid; an anonymous union may only have public non-static data members [-fpermissive]

[ZehMatt]

I can keep the struct names but that would require to move them out of the union.

[athre0z]

Ah. Yeah, we switched to C11 to permit anonymous unions. Looks like this is an incompatibility between C++ and C11 then, meh. I think we might have to keep the structs named, but move them out of the nesting then instead of removing the names.

[ZehMatt]

Also you mentioned of possibly breaking bindings, how do you bind a type that comes from an anonymous struct/union, is it in C11 accessible from the global scope? I'm not that good with C but I would imagine the type can't be accessed, at least not in Cpp.

[athre0z]

I don't think it's possible in C11 either, but bindgens often essentially flatten nested types into global scope, so it works there. If the structs are unnamed, they'll essentially get something like a SHA1 hash as their name.

Thinking about it again, however, I remembered that we actually stopped using bindgens for the languages where it mattered, switching to hand-crafted struct types instead. Having these fields named thus doesn't actually matter too much anymore. While I do like having them named, I feel that unnesting them kind of makes discoverability of the struct a lot worse -- you have to follow all the "links" to find the fields you're interested in, so I guess your proposed solution here is preferable.

[ZehMatt]

I'm glad you commented before I pushed onto this branch, the alternative would be: https://github.com/ZehMatt/zydis/tree/fix-anonymous-types-2

[athre0z]

I'll say it's probably the lesser evil this way, but I'll wait for @flobernd to take a look as well before merging. Either way, thanks for bringing this to our attention and proposing the fix!

[flobernd]

@athre0z @ZehMatt Just found some minutes to check this. I'd highly prefer option 2 just to be on the safe side. I remember having some problems when working on the C# bindings. If you are fine as well @athre0z, maybe @ZehMatt you can create a PR for your other solution instead?

[ZehMatt]

@athre0z @ZehMatt Just found some minutes to check this. I'd highly prefer option 2 just to be on the safe side. I remember having some problems when working on the C# bindings. If you are fine as well @athre0z, maybe @ZehMatt you can create a PR for your other solution instead?

I can either open a new PR or push it onto this one, as you wish.

[flobernd]

@ZehMatt Sure, pushing it to this one is even better, thanks! Let's just wait for @athre0z to agree.

[athre0z]

Hmm, yeah, it's fine with me. I feel like the struct is easier to navigate when nested as-is, but if there are still bindings that depend on this, let's do it as suggested. I think it will also result in better documentation in doxygen, since it otherwise also generates hashed struct names.

[ZehMatt]

Alright pushed the other branch onto this one, the CI unfortunately won't run since this is my first PR.

[flobernd]

Thanks! I started the CI.

[flobernd]

LGTM. Just a minor style issue, but we can as well solve later.

[flobernd]

We should add some linebreaks (before all the struct comments).

[ZehMatt]

I agree, I wasn't sure if I should do it or not since I copied it out as is, I addressed that.

[flobernd]

Thanks a lot!

[flobernd]

LGTM!