D17858: clang: Also detect Clang builtin dirs at runtime on Unix

Milian Wolff noreply at phabricator.kde.org
Thu Jan 10 11:57:33 GMT 2019


mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  I'm fine with his minus the change to return an empty path as that breaks the error message
  
  the code comments talking about minor versions can probably just be removed - we just try to use whatever is thrown at us and if it fails, then so be it and shit happens ;-)
  
  if you really want to check only for minor updates, you'd have to compare (via `QVersionNumber`) the clang version to the one we used at compile time...

INLINE COMMENTS

> clangsupport.cpp:188
> +                                     "See also: https://bugs.kde.org/show_bug.cgi?id=393779",
> +                                     ClangHelpers::clangBuiltinIncludePath()));
>              return;

the path is empty, so nothing will be printed here

please revert to the old behavior and check the validity of the returned path in the conditional, then print builtinDir here

> clanghelpers.cpp:413
> +        // return empty string to signal failure
> +        return QString();
>      }();

don't change this (see above)

> clanghelpers.h:108
> + * The returned path is the env. var KDEV_CLANG_BUILTIN_DIR when set otherwise the path
> + * to the headers used when kdev-clang was built, possibly updated for minor upgrades to
> + * the library (e.g. 7.0.0 -> 7.0.1).

the code isn't actually only checking for minor upgrades, or?

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, flherne, mwolff
Cc: arrowd, mwolff, flherne, kdevelop-devel, glebaccon, hase, antismap, iodelay, geetamc, Pilzschaf, akshaydeo, surgenight
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190110/0e51b221/attachment.html>


More information about the KDevelop-devel mailing list