[cfe-commits] r125615 - /cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

[Douglas Gregor]

On Feb 15, 2011, at 4:40 PM, Devang Patel wrote:

> 
> On Feb 15, 2011, at 4:26 PM, Douglas Gregor wrote:
> 
>> 
>> On Feb 15, 2011, at 3:36 PM, Devang Patel wrote:
>> 
>>> Author: dpatel
>>> Date: Tue Feb 15 17:36:28 2011
>>> New Revision: 125615
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=125615&view=rev
>>> Log:
>>> Only c++ class arguments with non trivial constructor or destructor needs a reference. 
>>> C struct arguments do not need this adjustment.
>>> This fixes 7 failures in callfuncs.exp from gdb testsuite.
>>> 
>>> Modified:
>>>  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>> 
>>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=125615&r1=125614&r2=125615&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Feb 15 17:36:28 2011
>>> @@ -1761,7 +1761,7 @@
>>> // If an aggregate variable has non trivial destructor or non trivial copy
>>> // constructor than it is pass indirectly. Let debug info know about this
>>> // by using reference of the aggregate type as a argument type.
>>> -  if (IndirectArgument && VD->getType()->isRecordType())
>>> +  if (IndirectArgument && VD->getType()->isClassType())
>>>   Ty = DBuilder.CreateReferenceType(Ty);
>> 
>> This doesn't do what the comment says it does. If you want to actually check for non-trivial destructors and non-trivial copy constructors, you need to check the class declaration directly:
>> 
>> if (IndirectArgument)
>> if (CXXRecordDecl *Record = VD->getType()->getAsCXXRecordDecl())
>>   if (!Record->hasTrivialCopyConstructor() || !Record->hasTrivialDestructor())
>> 	Ty = DBuilder.CreateReferenceType(Ty);
> 
>> 
>> 
>> That said, is the comment even right? It seems like the condition we really want here is whether it's a non-POD class type from the ABI's perspective. 
>> 
>> 	- Doug
> 
> IndirectArgument is only true when ABIArgInfo determines that argument is passed indirectly. This covers if (!Record->hasTrivialCopyConstructor() || !Record->hasTrivialDestructor()), check. I think it is appropriate to listen to ABIArgInfo.

Yes, ABIArgInfo is the point of truth.

> I was in the impression that c structs would also need this treatment, but I was wrong. 

C structs aren't passed indirectly. But isClassType() doesn't seem like the right condition to check, since a C++ struct can have a non-trivial copy constructor or destructor and therefore be passed indirectly.

	- Doug