dblaikie added a comment. Will wait on this 'til next Monday at least. Wouldn't mind a double-check/confirmation from @rjmccall that the Darwin checking is suitable (since I didn't see any other Darwin checking in RecordLayoutBuilder).
================ Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1891 + llvm::Triple Target = Context.getTargetInfo().getTriple(); + bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() || + Context.getLangOpts().getClangABICompat() <= ---------------- rsmith wrote: > dblaikie wrote: > > rsmith wrote: > > > `isPOD` is C++ standard specific, and our ABI rule really shouldn't be. > > > Does GCC use the C++98 rules here, the C++11 rules, or something else? > > > (Hopefully the GCC behavior doesn't change between `-std=c++98` and > > > `-std=c++11`!) > > > > > > From a quick test, it looks like GCC doesn't pack fields whose types are > > > classes with base classes, even if they're trivially-copyable and > > > standard-layout, suggesting that it's probably using the C++98 POD rules. > > I /think/ `CXXRecordDecl::isPOD` doesn't use a language-varying definition, > > according to its documentation at least: > > https://github.com/llvm/llvm-project/blob/1e3a02162db20264e9615b1346420c8d199cb347/clang/include/clang/AST/DeclCXX.h#L1124 > > & the code that sets the field that the accessor returns looks, to me at > > least, consistent with that claim: > > https://github.com/llvm/llvm-project/blob/1e3a02162db20264e9615b1346420c8d199cb347/clang/lib/AST/DeclCXX.cpp#L983 > > > > But if there's another way I should spell this to make it more > > clear/correct, more test cases to add to show the difference between these > > definitions - I'm open to that... > Ah, right you are, I was thinking of `QualType::isPODType`, which does depend > on the language mode: > https://github.com/llvm/llvm-project/blob/1e3a02162db20264e9615b1346420c8d199cb347/clang/lib/AST/Type.cpp#L2350 > > No action necessary here, I think. Cool - reckon it's worth renaming it to be more clear about which POD it represents? `IsClassicPOD`, `isRetroPOD`, `isCXX03POD`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117616/new/ https://reviews.llvm.org/D117616 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
