Why are empty Indexed*Identifier checked for ref counting?

[David Nolden]

2010/5/5 Milian Wolff <mail at milianw.de>:
> On Wednesday, 5. May 2010 20:00:42 David Nolden wrote:
>> 2010/5/5 Milian Wolff <mail at milianw.de>:
>> > Hey all, esp. David:
>> >
>> > Why are IndexedQualifiedIdentifier and IndexedIdentifier checking
>> > shouldDoReferenceCounting when they are empty? I.e. their ctors without
>> > any arguments.
>>
>> Because they still have an item assigned, the "empty" one, so also
>> reference-counting has to be done for it. This is the "correct" way to
>> do it, and is consistent with the way an indexed identifier is
>> converted into a real identifier: Based on the index, it retrieves the
>> item from the item-repository (see Identifier::Identifier(uint
>> index)).
>>
>> I've also thought a few times to treat empty identifiers by giving
>> them a "zero" index instead of the
>> emptyConstantIdentifierPrivateIndex() thing, but that would need a lot
>> of special handling, also in the function mentioned above.
>
> What would be more special compared to IndexedString that already uses index
> == 0 for empty indizes?
The difference is that for the empty string, we don't need the "item"
from the repository, while for the empty identifier we need.

>> > And shouldn't/couldn't we use m_index(0) for the empty case, just like
>> > with IndexedStrings? That would also speed up KDevelop, since right now
>> > e.g. the IndexedQualifiedIdentifier in the CodeModel always goes through
>> > this no matter what. The question is of course whether it should do
>> > duchainrefcounting at that point or not ;-)
>>
>> See above, the problem is that at some point we need to convert the
>> index to the item through the item repository.
>>
>> Regarding the next message:
>> Yes of course, reference-counting is only done for stuff that is being
>> saved to the disk, that's the interesting part about it this reference
>> counting, and also the reason why the reference-counting for the empty
>> identifiers you mentioned should be pretty irrelevant, because mostly
>> shouldDoDUChainReferenceCounting should do nothing more than
>> "if(false) return;".
>
> It's still in the top then of most expensive things in the call graphs
> though...
Not in any of the profilings that I did..

> Also: such things would really help in the doc comments. You might find them
> trivial but anyone else outside your brain needs to come up with your
> reasoning ;-)
>
> So if possible, please add some documentation.
Well, I even blogged about this. It's a basic concept of the
persistent duchain, and it is used throughout all the duchain storage
mechanisms. I'm also quite sure that there is some place where it's
explained in a comment.

Greetings, David