Why are empty Indexed*Identifier checked for ref counting?

[Milian Wolff]

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?

> > And actually I'd be interested in why we need something like
> > emptyConstantIdentifierPrivateIndex() at all. It uses e.g.:
> >
> > static uint index =
> > identifierRepository->index(DynamicIdentifierPrivate()); if (index == 0)
> >  ...
> >
> > the doc comment of ->index tells us, that it can _never_ be 0, hence
> > either the conditional is obsolete or the comment is wrong.
> 
> I don't remember the reason for that "index == 0" thing, sounds like
> something that was added temporarily while debugging and somehow
> creeped in.

I'll remove it then.

> > 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...

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.
-- 
Milian Wolff
mail at milianw.de
http://milianw.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20100505/9102e1a4/attachment.sig>