[ms-cxxabi] Set proper SRet flags for most functions; also handle empty struct arguments correctly

[Timur Iskhodzhanov]

2013/3/26 Timur Iskhodzhanov <timurrrr at google.com>:
> 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.
I went forward and fixed the byval issues I've found, see the attached patch.
It also fixes the "empty struct" problem easier than it did before.

I don't claim full compatibility there, but it's a two-liner step forward.
I'm fine to split the patch into "return-only" and "argument-only"
changes if you think that's appropriate.

>> 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_3.patch
Type: application/octet-stream
Size: 9817 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130326/0769be49/attachment.obj>