[cfe-dev] SelectorTable and IdentifierTable

[steve naroff]

On Jan 20, 2009, at 6:17 PM, Ted Kremenek wrote:

> On Jan 20, 2009, at 5:53 PM, steve naroff <snaroff at apple.com> wrote:
>
>>
>> On Jan 20, 2009, at 5:23 PM, Ted Kremenek wrote:
>>
>>> Is there a specific reason why we need both IdentifierTable and
>>> SelectorTable?  Why not just have IdentifierTable use  
>>> SelectorTable as
>>> an internal data structure, and have all selector creation go  
>>> through
>>> IdentifierTable?  It seems to buy us nothing that have two separate
>>> concepts from the perspective of clients.
>>> ______
>>
>> Hey Ted,
>>
>> What's the benefit of unifying them? (other than removing some API/ 
>> code).
>>
>> Having two separate table's that model the C/ObjC namespaces may be  
>> better from a lookup perspective (fewer hash collisions). There may  
>> be some size reduction I imagine (however I believe it would be  
>> quite small).
>>
>> Unless you see a performance win, I'm not sure fiddling with this  
>> level of clang is important (for now, at least).
>
> I wasn't thinking of combining their lookup tables.  That isn't even  
> possible; one uses a StringMap and the other a FoldingSet.  What I  
> was thing was:
>
> 1) Make SelectorTable an internal implementation class of  
> IdentifierTable.
>
> 2) Have the getSelector() methods be in IdentifierTable.
>
> 3) Allocate all MultiKeywordSelector objects using the same  
> BumpPtrAllocator used by IdentifierTable.  Currently these objects  
> are being malloc'ed and then always leaked (~SelectorTable doesn't  
> free them).
>

Got it (I thought you were talking about combining the lookup tables).

> Conceptually both IdentifierTable and SelectorTable reason about  
> identifiers.  Why not just have one IdentifierManager?

Since many selectors contain colons, I don't think of them as  
identifiers (since C identifiers can't contain colons).

> SelectorTable is basically a shim on top of IdentifierTable anyway.   
> Combining the two makes the API a little simpler since clients don't  
> have to create yet another book-keeping object that they must  
> properly dispose.

I agree. I think combining them makes sense, I just don't see it as a  
big win (since not many clients use the SelectorTable API).

snaroff