[PATCH] D41039: Add support for attribute "trivial_abi"

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.


================
Comment at: lib/Sema/SemaDecl.cpp:11700
+    }
+  }
+
----------------
ahatanak wrote:
> rsmith wrote:
> > rjmccall wrote:
> > > I think it's correct not to call CheckDestructorAccess and DiagnoseUseOfDecl here, since according to the standard destructor access is always supposed to be checked at the call-site, but please leave a comment explaining that.
> > The corresponding code for `areArgsDestroyedLeftToRightInCallee` is in `SemaChecking`. This should be done in the same place.
> > 
> > More generally, we have at least three different places where we check `CXXABI::areArgsDestroyedLeftToRightInCallee() || Type->hasTrivialABIOverride()`. It would make sense to factor that out into a `isParamDestroyedInCallee` function (probably on `ASTContext`, since we don't have anywhere better for ABI-specific checks at a layering level that can talk about types).
> I defined function ASTContext::isParamDestroyedInCallee and used it in two places. I didn't use it in CodeGenFunction::EmitParmDecl because the destructor cleanup is pushed only when the argument is passed indirectly in MSVC's case, whereas it is always pushed when the class is HasTrivialABIOverride.
I was misunderstanding the code in CodeGenFunction::EmitParmDecl. When the argument is a class that is passed by value, Arg.isIndirect() returns true even when the argument is passed directly. The code that pushes the destruction cleanup should remain in the if part of the if statement and isParamDestroyedInCallee can be used to determine whether the parameter is destructed in the callee.


https://reviews.llvm.org/D41039