[clang] [clang][CGRecordLayout] Remove dependency on isZeroSize (PR #96422)

[Michael Buch via cfe-commits]

Michael137 wrote:

> > > For CGClass, it's not directly tied to the LLVM structure layout, but I'm not sure the generated code would be semantically correct if an "empty" field that isn't isEmpty() overlaps with actual data.
> > 
> > 
> > I haven't addressed this yet. To clarify, are you referring to `addMemcpyableField` and `PushCleanupForField` (both of which check `isZeroSize`? So if we generated an overlap between an "empty" (but not `isEmpty`/`isZeroSize`) field and a non-empty field, previously `addMemcpyableField` wouldn't have considered the field for inclusion in the memcpy? So we should adjust the `isZeroSize` here too? Doing so didn't reflect in the test-suite, so it's possible there's some missing coverage here too
> 
> In addMemcpyableField(), if we conclude a field is not zero-size (and the other heuristics pass), we generate a memcpy for it. But if the field is actually empty, it might overlap with some other field, so the memcpy() might actually end up overwriting something unrelated. Similarly, for PushCleanupForField, we'll poison any overlapping data. In both cases, if the field doesn't actually contain any data, we can safely skip the relevant code.
> 
> I wouldn't be surprised if there's missing coverage here.

Makes sense!

> It looks like in the latest patch, there's still a couple uses of isZeroSize() in CGExprConstant?

Yea those were the last occurrences I wanted to sanity check since they're calling into `ASTRecordLayout::getFieldOffset` (which is built based on `isZeroSize`) and there's a `NoUniqueAddressAttr` check in there too. But it seemed reasonable to skip empty fields here too. Had to adjust the `CodeGen/2009-06-14-anonymous-union-init.c` test which checks for a `zeroinitializer` (which in the test happened to be for an empty structure, see latest commit).

https://github.com/llvm/llvm-project/pull/96422