[cfe-dev] Selector::getAsString problem in CIndex

[steve naroff]

On Oct 20, 2009, at 4:25 PM, Fariborz Jahanian wrote:

>
> On Oct 20, 2009, at 12:43 PM, steve naroff wrote:
>
>> Hi John,
>>
>> The patch didn't apply for me (it's based on yesterday's source).  
>> Could you update it?
>>
>> On the string lifetime issue...after consulting with Daniel, we'd  
>> prefer to avoid cluttering up the API to deal with lazily computed  
>> names (that aren't part of the AST per se).
>>
>> One spin on your change...
>>
>> +// For temporary string returns.
>> +static char CIBuffer[256];
>> +
>>
>> ...is to have a static llvm::StringSet that contains copies of the  
>> strings. For example:
>>
>> static llvm::StringSet<> LazilyComputedShortLivedNames;
>>
>> While this is more general than your change (no limit, can hold an  
>> arbitrary # of strings), it is static data (which is less than  
>> great).
>>
>> It would be nice if this data were cached per-Index. Unfortunately,  
>> this would require either (a) more bookkeeping or (b) more explicit  
>> 'CXIndex' arguments be passed around.
>>
>> So...it seem like the choices are:
>>
>> (a) static StringSet<> to keep the API simple (i.e. no requirement  
>> on the client to size/deal with strings).
>> (b) per-index StringSet<> to keep the string API simple. Modify the  
>> API to pass CXIndex in more places.
>> (c) per-index StringSet<> to keep the string API simple. Add  
>> bookkeeping to navigate from a CXDecl->CXTranslationUnit->CXIndex.
>> (d) Avoid passing back C strings entirely (what I previously said  
>> we'd like to avoid). Complicates the API, but makes the contract  
>> very explicit.
>> (e) Go with your change (documenting the limitations).
>>
>> At the moment, this is primarily a problem with ObjC names (i.e.  
>> Selectors), however this will be an issues for C++ as well I  
>> believe (for names that contain type info).
>>
> If the names, such as selectors, are globally unique, it makes sense  
> to go with a). Unless, per-index naming is needed for functionality  
> reasons.
>

Makes sense, however it's not just functionality.

I'm also concerned about reclaiming the string pool. If it's static, I  
can't reclaim it. If it's per-index, clang_disposeIndex() could  
reclaim it.

snaroff

> - Fariborz
>