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

[Fariborz Jahanian]

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.

- Fariborz