richard.townsend.arm added inline comments.
================ Comment at: lib/Sema/SemaDeclCXX.cpp:5908 + // The checks below ensure that thare no user provided constructors. + // For AArch64, we use the C++14 definition of an aggregate, so we also + // check for: ---------------- I think these checks are in the wrong place - the aggregate restriction applies to return values, but it doesn't apply to arguments. Placing this logic here breaks argument passing between MSVC and Clang, because Clang (with this logic in place) will pass and expect arguments with an extra layer of indirection: for example, when passing https://cs.chromium.org/chromium/src/v8/include/v8.h?q=v8::Local&sq=package:chromium&g=0&l=183 v8::Local, what you actually see in the register bank is supposed to be its only private member. Moving the logic to MicrosoftCXXABI and applying it only to the return value resolves this problem. ================ Comment at: lib/Sema/SemaDeclCXX.cpp:5916 + if (isAArch64) { + if (D->getAccess() == AS_private || D->getAccess() == AS_protected) + return false; ---------------- efriedma wrote: > D->getAccess() is the access specifier for the class itself (if the class is > nested inside another class), not the access specifier for any of its members. > > I think you're looking for HasProtectedFields and HasPrivateFields. (I think > there currently isn't any getter on CXXRecordDecl for them at the moment, but > you can add one.) So the issue that I mentioned in https://bugs.llvm.org/show_bug.cgi?id=41135#c18 is resolved by checking HasProtectedFields/HasPrivateFields. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60349/new/ https://reviews.llvm.org/D60349 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
