D5491: Improve detection of builtin defines from compiler command

[Milian Wolff]

mwolff added a comment.


  excellent test coverage, much appreciated, some nitpick notes

INLINE COMMENTS

> gcclikecompiler.cpp:41
>  
> -QStringList languageOptions(const QString& arguments)
>  {

make the diff easier to read by keeping the old function as a per-argument version and introduce a new function that only adds the loop over the args?

> gcclikecompiler.cpp:103
>          }
> -        return {standard, language};
> -
> +        qCWarning(DEFINESANDINCLUDES) << "Lang+Standard for" << arguments << "are:" << standardFlag << language;
> +        result.append(language);

does this need to be a warning?

> gcclikecompiler.cpp:115
>      }
> +    // FIXME: thread safety???
>  

? what are you worried about here?

> gcclikecompiler.cpp:127
> +    auto compilerArguments = languageOptions(splitArguments);
> +    compilerArguments.append("-dM");
> +    compilerArguments.append("-E");

QStringLiteral, also below

> gcclikecompiler.cpp:155
>  
> +    // FIXME: thread safety???
> +

dito

> gcclikecompiler.cpp:175
> +    auto compilerArguments = languageOptions(splitArguments);
> +    compilerArguments.append("-E");
> +    compilerArguments.append("-v");

dito

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D5491

To: arichardson, #kdevelop, kfunk, mwolff
Cc: aaronpuchert, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170418/0aaf48ad/attachment-0001.html>