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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to