About FunctionDeclaration and ClassFunctionDeclaration

[Milian Wolff]

On Friday 09 March 2012 00:42:18 Sven Brauch wrote:
> Hi,
> 
> those two classes are causing major trouble for me currently. I
> noticed that the class browser expects all class member functions to
> be instances of ClassFunctionDeclaration, and other functions to be
> FunctionDeclarations -- so I had to change that in python in order to
> make the class browser work correctly. In the first place, this
> doesn't make sense for python, as whether a function is a class member
> or not is not relevant at all. If I neverthereless adopt those two
> classes, I can't easily access properties of a function declaration!
> For example, I used this pattern quite a few times:
> 
>   Declaration* d = ...
>   if ( d->isFunctionDeclaration() ) my_type =
> static_cast<FunctionDeclaration*>(d)->type(); // or returnType, or
> parametersCount, or... whatever
> 
> If I have to differentiate between ClassFunctionDeclaration and
> FunctionDeclaration now, this expands to
> 
>   Declaration* d = ...
>   if ( d->isFunctionDeclaration() ) {
>     if ( FunctionDeclaration* f =
> dynamic_cast<FunctionDeclaration*>(d) ) my_type = f->type();
>     if ( ClassFunctionDeclaration* f =
> dynamic_cast<ClassFunctionDeclaration*>(d) ) my_type = f->type();
>   }
> 
> now. This is ugly, slow, and unreadable.

I doubt it's *that* slow. Agree on the ugly + unreadable side though :) You 
could of course write a helper function, eh? ;-)

> I also can't use AbstractFunctionDeclaration, it doesn't even have
> type() or similar.
> 
> IMO the proper way to fix this would be to have an isClassMember()
> property in AbstractFunctionDeclaration, and have the class browser
> check for that instead of dynamic_casting its way around [1]. Then
> every language could do whatever it wants with that flag, and
> ClassMemberDeclaration could have it set by default so the current
> behaviour won't change. What do you think?
> Alternatively, I think it would make sense to have
> ClassFunctionDeclaration inherit FunctionDeclaration. The code of
> ClassFunctionDeclaration duplicates FunctionDeclaration's code
> currently anyways (for example defaultParameters(), etc). Then again,
> I don't really understand the duchain class hierarchy well enough to
> suggest such a change. :)

This is indeed a sore spot yet I don't see many possibilities to improve the 
situation and indeed think that it's actually resolved quite nicely. One could 
think about putting more of the code from FunctionDeclaration & 
ClassFunctionDeclaration into the AbstractFunctionDeclaration, like the 
parameter stuff. Your case won't be changed by that (you'd still need to 
subclass both and add your custom data twice).

For the class browser, we could think about removing the dynamic cast and just 
use class->localDeclarations & isFunctionDeclaration to find methods. After 
all, every function inside a class context, should by definition be a method, 
no? You could try that out and see whether it breaks php/cpp class browser. If 
not, why not use that? Apparently this was not an issue in other places so far 
for you, or?

> ___________
> [1] PS: While we're at that topic, it would be great to have a few
> more of those functions anyways, to avoid dynamic_cast type checking.
> For example, isClassDeclaration() would be useful for me. Is there any
> general objection in adding such things?

If it's worth it... Personally I rarely have a case where I just need to know 
what kind a declaration is, most often I'll also have to call one of the 
methods of the more concrete type. Hence something like:

if(c = dynamic_cast<...>(dec)) {
 c->foo();
}

Adding an additional virtual function call here just to save one dynamic cast 
in some conditions... I doubt this is much faster. And of course not more 
readable! I'd like to see some benchmark that shows how the dynamic cast 
compares to the virtual function call :)

bye

-- 
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/20120309/09a4c1ce/attachment.sig>