Review Request 117938: Support for default include path and definitions on CLang compiler.

[Kevin Funk]

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



languages/cpp/setuphelpers_gcc_like.cpp
<https://git.reviewboard.kde.org/r/117938/#comment39883>

    Make 'compiler' the first argument.
    Please no 'const' for PODs.
    
    Also rather call the function 'computeGccLikeStandardIncludePaths' (similar to the other one for macros)



languages/cpp/setuphelpers_gcc_like.cpp
<https://git.reviewboard.kde.org/r/117938/#comment39882>

    That won't work for non-unix-like paths, like on Windows.
    
    The old parsing code here is that complex for a reason. I suggest you to just keep the old parsing code in gccLikeSetupStandardIncludePaths (but make it possible to specify the compiler like here).



languages/cpp/setuphelpers_gcc_like.cpp
<https://git.reviewboard.kde.org/r/117938/#comment39889>

    This can be simplified.
    
    foreach (const QString& compiler, SUPPORTED_COMPILERS) {
        const QStringList includePaths = gccLikeSetupStandardIncludePaths(compiler, true);
        if (!includePaths.isEmpty())
             return includePaths;
    }
    
    kDebug() << ...;
    return {};



languages/cpp/setuphelpers_gcc_like.cpp
<https://git.reviewboard.kde.org/r/117938/#comment39885>

    Again, make 'compiler' the first param.



languages/cpp/setuphelpers_gcc_like.cpp
<https://git.reviewboard.kde.org/r/117938/#comment39884>

    Minor: Remove excessive whitespace (remove newline)



languages/cpp/setuphelpers_gcc_like.cpp
<https://git.reviewboard.kde.org/r/117938/#comment39886>

    Not used.



languages/cpp/setuphelpers_gcc_like.cpp
<https://git.reviewboard.kde.org/r/117938/#comment39887>

    Just call that var 'line'



languages/cpp/setuphelpers_gcc_like.cpp
<https://git.reviewboard.kde.org/r/117938/#comment39888>

    Minor: Remove newline



languages/cpp/setuphelpers_gcc_like.cpp
<https://git.reviewboard.kde.org/r/117938/#comment39890>

    Use the same pattern as suggested in gccLikeSetupStandardIncludePaths


And: Please remove extra whitespace in empty lines in your patch.

- Kevin Funk


On May 3, 2014, 9:54 p.m., Luis Felipe Dominguez Vega wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117938/
> -----------------------------------------------------------
> 
> (Updated May 3, 2014, 9:54 p.m.)
> 
> 
> Review request for KDevelop and Luis Felipe Dominguez Vega.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> When I compile Kdevelop using only in my Debian Testing Clang 3.4, the defaults include paths is not recognized. In the code I will see that the functions are only for GCC and MSVC, with this patch now with Clang are recognized both, the include paths, and standard definitions.
> 
> 
> Diffs
> -----
> 
>   languages/cpp/CMakeLists.txt 5add7c0 
>   languages/cpp/setuphelpers.cpp 7481256 
>   languages/cpp/setuphelpers_gcc.cpp 83c6052 
>   languages/cpp/setuphelpers_gcc_like.cpp PRE-CREATION 
>   languages/cpp/tests/CMakeLists.txt dcf5087 
> 
> Diff: https://git.reviewboard.kde.org/r/117938/diff/
> 
> 
> Testing
> -------
> 
> Tested in my own PC, Debian Testing 64 bits, Clang 3.4, Kernel "Linux 3.13.10lfdominguez #1 SMP PREEMPT Sat Apr 26 17:56:25 UTC 2014 x86_64 GNU/Linux"
> 
> 
> Thanks,
> 
> Luis Felipe Dominguez Vega
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140504/760a08e7/attachment-0001.html>