On Wed, Jul 10, 2013 at 10:01 PM, Eli Friedman <[email protected]>wrote:
> Please split out the SemaDecl.cpp changes into another patch. > > Some of the changes to test/Sema/ms_class_layout.cpp don't look right; > are you sure they're accurate? > > > * It's still in two files and doesn't use CRTP. The net overlap > between the current record builder and this one is 7 member variables, 1 > typedef and 10 functions with the same names but different functionality. > The closest functionality is laying out fields (non-bit-fields) but it > rounds in a slightly different place and is only a few lines of code. > Basically although in spirit the ms and non-ms builders achieve similar > goals, they do so in completely different ways. I would use an analogy of > implementing a dictionary using a B-tree or a red-black tree. They may > both implement a dictionary using a tree but don't actually have much of > any share-able code. > > It's also worth noting that you didn't implement -Wpadded... > I would argue that -Wpadded should be in Sema operating on the layouts produced by AST, but it looks like the field sizes aren't recorded in the layout and recomputing them is not trivial.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
