eugenis accepted this revision. eugenis added a comment. This revision is now accepted and ready to land.
LGTM ================ Comment at: clang/lib/CodeGen/CGClass.cpp:1742 + Context.toCharUnitsFromBits(Layout.getFieldOffset(layoutStartOffset) + + Context.getCharWidth() - 1); + llvm::ConstantInt *OffsetSizePtr = ---------------- vitalybuka wrote: > vitalybuka wrote: > > morehouse wrote: > > > How does this interact with bit fields? > > I guess existing code work because is bit fields are trivial and the one > > aligned to the char is going to be layoutStartOffset > > I assume if we call this function for layoutStartOffset pointing to the > > bitfield in the middle of char it will poison entire byte which is > > incorrect. > > With the patch it will not poison such chars: now it will be either all > > bits or nothing which I believe better. > toCharUnitsFromBits already down aligns by definition: it's just a > bitsoffset/charwidth > > regarding assert: I am not 100% that this is not possible, maybe some > corner-case in language allows that. > if so, it's better not to poison partial byte at all than poison it > completely. > > I assume if we call this function for layoutStartOffset pointing to the > bitfield in the middle of char it will poison entire byte which is incorrect. Right. We never do this, of course, because the previous non-trivial field can not possibly end in the middle of a char. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92728/new/ https://reviews.llvm.org/D92728 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
