[PATCH] D53497: [AST] Do not align virtual bases in `MicrosoftRecordLayoutBuilder` when an external layout is used

Reid Kleckner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 22 10:21:19 PDT 2018


rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

> Also it seems that MicrosoftRecordLayoutBuilder with an external source and without one differ considerably, may be it is worth to split this and to create two different builders? I think that it would make the logic simpler. What do you think about it?

Yes, I recall when reading the Itanium record layout code that it was riddled with extra checks for external layouts. Back then I thought it would be nicer to have a separate code path for external layouts. I appreciated that when we separated out the MS code, it was clean and not filled with these checks. But, of course, now here we are adding them in. If you want to do the work to come up with a separate, more minimal code path that produces ASTRecordLayouts from external layouts, I'd definitely help review it.

This change is pretty small and targeted, so feel free to land it as is first. Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D53497





More information about the cfe-commits mailing list