arichardson added inline comments.
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1917
+ EffectiveFieldSize = FieldSize = TI.Width;
+ FieldAlign = TI.Align;
} else {
----------------
rjmccall wrote:
> This is a nice cleanup, but I actually can't figure out why we can't just
> fall into the code below.
It looks like this dates back to `Add a new ASTRecordLayoutBuilder class. Not
used yet.` from 2009. Maybe `ReferenceType`s were not handled correctly by
`Context.getTypeInfoInChars(RT);` back then?
================
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1917
+ EffectiveFieldSize = FieldSize = TI.Width;
+ FieldAlign = TI.Align;
} else {
----------------
arichardson wrote:
> rjmccall wrote:
> > This is a nice cleanup, but I actually can't figure out why we can't just
> > fall into the code below.
> It looks like this dates back to `Add a new ASTRecordLayoutBuilder class. Not
> used yet.` from 2009. Maybe `ReferenceType`s were not handled correctly by
> `Context.getTypeInfoInChars(RT);` back then?
I've removed the special case now and it looks like all tests are passing, so I
believe this is indeed no longer needed.
================
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1706
+ assert(ThisPtrTy->getPointeeType().getAddressSpace() == LangAS::Default &&
+ "this pointer not in default address space?");
auto Align = getTypeAlignIfRequired(ThisPtrTy, CGM.getContext());
----------------
rjmccall wrote:
> I'm pretty sure we have language modes that support methods in different
> address spaces, and your code doesn't seem to require this assertion.
I added this assertion to see if we have any tests for this case and forgot to
remove it. I think it should hopefully be correct, so I've removed the
assertion.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138295/new/
https://reviews.llvm.org/D138295
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
