[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

Eli Friedman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 22 16:24:37 PDT 2019


efriedma added inline comments.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:5904
 
   if (CCK == TargetInfo::CCK_MicrosoftWin64) {
+    bool isAArch64 = S.Context.getTargetInfo().getTriple().isAArch64();
----------------
I'm not entirely sure it makes sense to do all of these Windows-specific checks here... I think it makes trivial_abi not work the way it should.  But that's not something we need to worry about for the initial patch, I think.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:5916
+    if (isAArch64) {
+      if (D->getAccess() == AS_private || D->getAccess() == AS_protected)
+        return false;
----------------
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.)


================
Comment at: lib/Sema/SemaDeclCXX.cpp:5922
+        return false;
+      if (D->needsImplicitCopyAssignment() && !D->hasTrivialCopyAssignment())
+        return false;
----------------
Why are you checking needsImplicitCopyAssignment()?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60349/new/

https://reviews.llvm.org/D60349





More information about the cfe-commits mailing list