Review Request 120086: Fix cursorSpellingNameRange range for out-of-line functions.

[Sergey Kalinichev]

> On Sept. 7, 2014, 1:10 p.m., Milian Wolff wrote:
> > Sergey, can you explain why this is an issue for out-of-line methods? Is the Identifier for e.g. "foo::bar" not "bar", but something else?  Or is the range start column pointing to "foo" instead of "bar" and then adding the length of the Identifier to it makes it wrong?
> 
> Milian Wolff wrote:
>     note: when it's the latter, e.g. if the scope is wrongly taken into account, is there a way to change the clang_Cursor_getSpellingNameRange call to start at the "bar"?
> 
> Sergey Kalinichev wrote:
>     Example:
>     class foo {
>      void bar();
>     };
>     // Here is the id.toString() == "foo::bar", range start column at bar
>     void foo::bar(){
>     }
>     
>     So, if I understand you right the id.toString() should be just "bar"? And it should be fixed in some other place then?
> 
> Olivier Jean de Gaalon wrote:
>     See TUDUChain::makeId, where kdev-clang special cases CXXMethod identifiers. I was not aware this is done. Oldcpp did the same thing, but I'd like to find a better solution; maybe creating helper contexts with the class identifier.
> 
> Milian Wolff wrote:
>     The Identifier should be "foo", the QualifiedIdentifier otoh should be "foo::bar".
> 
> Milian Wolff wrote:
>     It might be required to use a QualifiedIdentifier in more places instead of Identifier. Then we can detect this here, and only extend the range of the use by the last element of the QualifiedIdentifier. The best solution of course would be to fix this upstream in clang... Which was done already (afaik?) :) So maybe we can do a costly workaround and ifdef that based on the clang version?

Hmm, I'm just wondering why is this CXXMethod identifiers workaround in TUDUChain::makeId needed at all?  
I've just tested it without it. Everything works fine with out-of-line methods: i.e. code-navigation, code highlighting, uses count, rename features e.t.c.
So, it's not clear for me why is it needed at all, except "that's how oldcpp does it". Maybe it just workarounds it's own bugs that way?

Also note, that currently rename capabilities don't work for out-of-line methods (actually this patch doesn't fix it too). But removing that CXXMethod identifiers workaround fixes it.

So, I'm not sure if we should do it like the oldcpp does it. Maybe it'd be better to just just remove it?


- Sergey


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120086/#review65956
-----------------------------------------------------------


On Sept. 7, 2014, 10:20 a.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120086/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2014, 10:20 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> This was accidentally broken by:
> f17866db1bc24c198e87c028065796959f6dd135
> Author: Kevin Funk <kfunk at kde.org>
> Date:   Mon Aug 4 16:53:06 2014 +0200
> 
>     Also fix ranges for operator functions
> 
> 
> Diffs
> -----
> 
>   duchain/clanghelpers.cpp fb69a66 
>   tests/files/functiondefinitiondeclarations.cpp fe4050e 
> 
> Diff: https://git.reviewboard.kde.org/r/120086/diff/
> 
> 
> Testing
> -------
> 
> All tests pass.
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140907/1fded4cd/attachment.html>