[ms-cxxabi] Set proper SRet flags for most functions; also handle empty struct arguments correctly
Timur Iskhodzhanov
timurrrr at google.com
Tue Mar 26 17:05:41 PDT 2013
2013/3/22 John McCall <rjmccall at apple.com>:
>
> On Mar 21, 2013, at 1:45 PM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>
>> Hi John,
>>
>> Please see the attached patch.
>>
>> It addresses most of the Clang-side change needed to fix http://llvm.org/PR13676
>
> + bool IsMicrosoftABI = getContext().getTargetInfo().getCXXABI().isMicrosoft();
> +
> if (isAggregateTypeForABI(RetTy)) {
> if (const RecordType *RT = RetTy->getAs<RecordType>()) {
> // Structures with either a non-trivial destructor or a non-trivial
> // copy constructor are always indirect.
> - if (hasNonTrivialDestructorOrCopyConstructor(RT))
> + if (hasNonTrivialDestructorOrCopyConstructor(RT) ||
> + (IsMicrosoftABI && hasNonTrivialDefaultConstructor(RT)))
>
> Please don't compute IsMicrosoftABI before it's needed.
Done.
> Are you sure it's just that the *default* constructor is non-trivial, or is it
> the presence of *any* non-trivial constructor, or possibly even any
> explicit constructor?
I've made some more experiments and it seems the condition is rather
"is this a POD?", e.g. we should use SRet if A has a virtual function
or a destructor or an assignment operator (but not "void
operator=(int);") or a private member or a base class.
See the attached updated patch.
I was a bit lazy to write tests for all these cases, will add them
later if we ever find new incompatibilities.
I've also added a couple of tests for the upcoming byval compatibility
cleanup with FIXMEs.
> For example, this class doesn't even have a default constructor:
> struct A {
> int x;
> A(int x) : x(x) {}
> };
>
> And this C++11 class has a trivial default constructor, but it does still have
> non-trivial constructors:
> struct A {
> int x;
> A() = default;
> A(int x) : x(x) {}
> };
>
> And this C++11 class has no non-trivial constructors, but its constructor is
> explicitly declared:
> struct A {
> int x;
> A() = default;
> };
This doesn't compile with VS2012, looks like it does not support the
default keyword...
> John.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bug_13676_p1_2.patch
Type: application/octet-stream
Size: 9340 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130326/a864a61d/attachment.obj>
More information about the cfe-commits
mailing list