[PATCH] D85191: [AST] Get field size in chars rather than bits in RecordLayoutBuilder.

Bevin Hansson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 4 08:43:11 PDT 2020


ebevhan added inline comments.


================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1841
   auto setDeclInfo = [&](bool IsIncompleteArrayType) {
-    TypeInfo TI = Context.getTypeInfo(D->getType());
-    FieldAlign = Context.toCharUnitsFromBits(TI.Align);
+    auto TI = Context.getTypeInfoInChars(D->getType());
+    FieldAlign = TI.second;
----------------
Xiangling_L wrote:
> In most cases, `getTypeInfoInChars` invokes `getTypeInfo` underneath. So to make people be careful about this, I would suggest to leave a comment explaining/claiming we have to call `getTypeInfoInChars` here. And also maybe adding a testcase to guard the scenario you were talking about would be helpful to prevent someone to use `getTypeInfo` here in the future.
I can do that.

I honestly don't think it would be a bad idea to add an assertion to toCharUnitsFromBits that checks for non-bytesize-multiple amounts. I wonder how much would fail if I did that, though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85191/new/

https://reviews.llvm.org/D85191



More information about the cfe-commits mailing list