[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