Disable the expansion of header unions in the copy structures pass.

[fruffy]

This expansion currently seems broken:
The pass expands something like

hdr.addr_dst = addr_0;

to

hdr.addr_dst.ipv4.setValid();
hdr.addr_dst.ipv4.addr = addr_0.ipv4.addr;
hdr.addr_dst.ipv6.setValid();
hdr.addr_dst.ipv6.addr = addr_0.ipv6.addr;

This assignment always invalidates ipv4, even though it may be valid. Some context: #4853 (comment)

[jafingerhut]

I am not sure, but it appears that these changes are attempting to fix the issue #3842 mentioned in comments just before this line, and thus those comments can be removed?

[fruffy]

Well, the current fix just disables union expansion by default to avoid running into #3842. Ideally, we fix the problem in the copyStructures pass somehow by tracking validity.

[jafingerhut]

Ah, this PR is not fixing that issue, if I understand the changes correctly. You are just disabling incorrect transformations.

[vlstill]

This seems to me like a reasonable interim solution. But the fix for the header union assignments should not be that hard (and it would not be less efficient than the current incorrect code).

[vlstill]

Seems unrelated. Also, it would be better to print the space only after the if (!isDeclaration).

[fruffy]

This is intentional. Fixes a bug introduced by #5036. If you do not change this line you will produce incorrect reference files. I am actually a little concerned why this is not picked up in checks.

[vlstill]

@fruffy, the old code is quite different from what you have written in the PR topic, do you have an example which produces such a code?

Also, this code is surprisingly close to what I would say is the correct (but inefficient on many targets) code.

        // or invalidate them one-by-one here to avoid need for target to be able to invalidate whole HU
        u_1[1].setInvalid();
        if (u_1[1w0].h1.isValid()) {
            u_1[1].h1.setValid();
            u_1[1].h1 = u_1[1w0].h1;
        }
        if (u_1[1w0].h2.isValid()) {
            u_1[1].h2.setValid();
            u_1[1].h2 = u_1[1w0].h2;
        }
        if (u_1[1w0].h3.isValid()) {
            u_1[1].h3.setValid();
            u_1[1].h3 = u_1[1w0].h3;
        }

The ifs can be else-if too, which would make it more explicit these are truly alternatives (in which case the setInvalid might be in a final else branch). The way I wrote it here it might be easier for some targets that for some reason don't invalidate other alternatives, but that is really a bug in the target as the u.hi.setValid() could be also written explicitly by the user.

The biggest problem in the original rewriting is the else branch which invalidates the HU if u_1[0].h3 is not valid. Given how close it is, it is probably worth it just fixing it.

[vlstill]

OK, based on a quick look in the code it might not be that simple, because we would need to change this if generation based on whether or not we are inside union as the current rewriting is the same for all headers, whether in structs or in unions -- which is why it is wrong. That said, it seems to me like CopyStructures in this case just expands the union (through it being StructLike) and then something else inserts the headers' if. If this was expanded by CopyStructures, it would be expanded to the level of fields of the header.