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

[David Chisnall]

On 2 Sep 2010, at 18:31, Devang Patel wrote:

> 
> On Sep 2, 2010, at 10:16 AM, David Chisnall wrote:
> 
>> Author: theraven
>> Date: Thu Sep  2 12:16:32 2010
>> New Revision: 112833
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=112833&view=rev
>> Log:
>> Use the unmangled name for the display name in Objective-C debug info.  This should have no effect with the Mac runtime where clang (unlike GCC) uses the display name symbol name.
>> 
> 
> test case ?

I'm not sure what the test case would test.

>> 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=112833&r1=112832&r2=112833&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Sep  2 12:16:32 2010
>> @@ -1471,6 +1471,20 @@
>>    Name = getFunctionName(FD);
>>    // Use mangled name as linkage name for c/c++ functions.
>>    LinkageName = CGM.getMangledName(GD);
>> +  } else if (const ObjCMethodDecl *OMD = dyn_cast<ObjCMethodDecl>(D)) {
>> +    llvm::SmallString<256> MethodName;
>> +    llvm::raw_svector_ostream OS(MethodName);
>> +    OS << (OMD->isInstanceMethod() ? '-' : '+') << '[';
>> +    const DeclContext *DC = OMD->getDeclContext();
>> +    if (const ObjCImplementationDecl *OID = dyn_cast<const ObjCImplementationDecl>(DC)) {
>> +       OS << OID->getName();
>> +    } else if (const ObjCCategoryImplDecl *OCD = dyn_cast<const ObjCCategoryImplDecl>(DC)){
>> +        OS << ((NamedDecl *)OCD)->getIdentifier()->getNameStart() << '(' <<
>> +            OCD->getIdentifier()->getNameStart() << ')';
>> +    }
>> +    OS << ' ' << OMD->getSelector().getAsString() << ']';
>> +    Name = MethodName;
> 
> Isn't this means Name will reference local SmallString MethodName outside MethodName's scope ?

Could be.  I'm not entirely clear about how LLVM's string stuff works - I assumed it was a copy that was optimised away - if so moving the MethodName declaration outside the block should fix it.

> Please use separate function here and if you need special treatment for GNU runtime then the code path should not be run for NEXT runtime.

The GNU runtime does not need special treatment here.  This is the correct behaviour for both runtimes.  By coincidence, the NeXT runtime code already generates the human readable name as the LLVM function name, however this is an artefact of the specific implementation in CGObjCMac - GCC on Darwin mangles the symbol names (I think clang would need to as well if it wanted to support older Darwin releases), so relying on the NeXT runtime code to do this does not seem like a good solution.  

If you want it in a separate function, where do you want it to live?  The NeXT runtime could use this same code for generating its symbol names, so it might be sensible to put it on the decl.  Alternatively, we could put it in the CGObjCRuntime code, but since the display name is exactly the same on all platforms (it depends on the source language, not on the implementation), that seems silly.

David