Review Request 123933: Set empty ranges for declarations inside macro expansion

Sergey Kalinichev kalinichev.so.0 at gmail.com
Fri May 29 18:02:46 UTC 2015



> On May 29, 2015, 1:42 p.m., Milian Wolff wrote:
> > duchain/builder.cpp, line 324
> > <https://git.reviewboard.kde.org/r/123933/diff/1/?file=378287#file378287line324>
> >
> >     add a comment:
> >     
> >     // check if cursor is inside a macro expansion
> >     
> >     also: what about nested macro expansions, are they OK to not get this treatment?

It's great that you pointed that out. Actually the CXCursor_MacroExpansion is a use, so it can never be here. My bad.


> On May 29, 2015, 1:42 p.m., Milian Wolff wrote:
> > tests/files/macros.cpp, line 1
> > <https://git.reviewboard.kde.org/r/123933/diff/1/?file=378288#file378288line1>
> >
> >     according to this test, it worked fine even before, no? I seem to have trouble understanding how the code below with the Cursor is related to uses inside a macro expansion. could you please elaborate?

At first I set empty ranges differently. All tests passed. Then I tried to test that patch manually and noticed that some uses couldn't be found anymore, so I investigated it and added this test case, then implemented the patch the way it is right now.


> On May 29, 2015, 1:42 p.m., Milian Wolff wrote:
> > tests/test_duchain.cpp, line 893
> > <https://git.reviewboard.kde.org/r/123933/diff/1/?file=378289#file378289line893>
> >
> >     hm this is not optimal, is it? shouldn't the range be that of the "x" macro argument? i.e. {1, 12, 1, 13}?

Here the "x" is not inside macro expansion so it has valid range (somehow it's actually {1, 14, 1, 15}), but the "strx" is inside the macro expansion so it has invalid range - {1, 0, 1, 14} (this is because clang gives the same range as macro expansion range for all declarations inside the macros expansion). If anything it should be something like (1,7), (1,10)

Also consider this example with the same problem. but without an argument to bind to: 
#define FOO_MACROS() int foo; 
FOO_MACROS()


- Sergey


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


On May 29, 2015, 12:34 p.m., Sergey Kalinichev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123933/
> -----------------------------------------------------------
> 
> (Updated May 29, 2015, 12:34 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> Those declarations are invisible, so there is no need to highlight and enable navigation widget for them.
> 
> 
> Diffs
> -----
> 
>   duchain/builder.cpp 260b4e2 
>   tests/files/macros.cpp 539afff 
>   tests/test_duchain.cpp 403e7c1 
> 
> Diff: https://git.reviewboard.kde.org/r/123933/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sergey Kalinichev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150529/42fdefcf/attachment.html>


More information about the KDevelop-devel mailing list