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

[Devang Patel]

On Feb 15, 2011, at 4:43 PM, Douglas Gregor wrote:

> 
> 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.

Good point. It seems I can avoid passing IndirectArgument here, even if it is the point of truth. I'll fix this soon.
-
Devang