Review Request 126889: Patch to remove/implement "TODO"

[Kevin Funk]

> On Jan. 26, 2016, 4:34 p.m., Kevin Funk wrote:
> > Did you test whether this test succeeds with both the old and the new implementation?
> > 
> > And: Nice to see you're working on this, Pedro!
> 
> Pedro Ferreira wrote:
>     Good call - my code wouln't build with the old version. I have fixed it now.
>     The tests, however, do not pass with the old version (ie, before updated clang). The "old" implementation returns the literal used to instanciate the template, "3" and "5". The first test passes.
>     I don't think that behaviour is correct, since "3" and "5" aren't types.

I see. Thinking about it, libclang minor version 24 was available before Clang version 3.5 was out. Since we depend on Clang 3.5 already, we can safely drop the feature check altogether.

-> Just use the new implementation, drop the old. But still keep the tests around, they're still useful!

Thanks


- Kevin


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


On Jan. 26, 2016, 5:21 p.m., Pedro Ferreira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126889/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2016, 5:21 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> This drops a TODO message and inserts a compile-time selector between clang-based template argument parsing and what appears to be manual tokenisation.
> 
> 
> Diffs
> -----
> 
>   languages/clang/codecompletion/completionhelper.cpp c01185a 
>   languages/clang/tests/test_clangutils.h 4508907 
>   languages/clang/tests/test_clangutils.cpp 1a8ecb7 
>   languages/clang/util/clangutils.h e47058e 
>   languages/clang/util/clangutils.cpp 81f7b44 
> 
> Diff: https://git.reviewboard.kde.org/r/126889/diff/
> 
> 
> Testing
> -------
> 
> This is possibly incomplete. It works for me (I'm using a KDev built with this), but would obviously appreciate comments.
> Mind you, the include header in Clang for this function ("clang_Type_getTemplateArgumentAsType") says that this function does not support variadic templates, so potentially this patch is not acceptable.
> 
> 
> Thanks,
> 
> Pedro Ferreira
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20160126/2a85dbe3/attachment.html>