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

Michael Buch via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 5 05:52:29 PDT 2024


Michael137 wrote:

Had to adjust `isEmptyFieldForLayout`/`isEmptyRecordForLayout` because `isEmptyField`/`isEmptyRecord` would call each other recursively, so dispatching from `isEmptyFieldForLayout` wouldn't have been correct (since the `isEmptyField`/`isEmptyRecord` have extra checks for unnamed bitfields and arrays which didn't make sense in the context of our emptiness check, correct me if I'm wrong here).

> 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

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


More information about the cfe-commits mailing list