efriedma-quic 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. 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
