About FunctionDeclaration and ClassFunctionDeclaration

[Sven Brauch]

Hi,

yeah, I also can't imagine a case where a function declaration is in a
class declaration's internalContext's localDeclarations(), but is not
a member function of that class. I'll test that out and file a review
request if it works.

I could write a helper function, but I'd need one helper function for
every property of FunctionDeclaration I want to access. Not very
elegant. :)

I don't think this issue occurs in other spots. Quickopen, tooltips
and everything work like expected, even if the member functions are
instances of FunctionDeclaration.

Greetings,
Sven

Am 9. März 2012 10:31 schrieb Milian Wolff <mail at milianw.de>:
> 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
> --
> KDevelop-devel mailing list
> KDevelop-devel at kdevelop.org
> https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel
>