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

Milian Wolff noreply at phabricator.kde.org
Thu Jan 10 08:41:38 GMT 2019

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

  one more round of cleanups please


> clanghelpers.cpp:374
>  {
>      static const auto dir = []() -> QString {
> +        const QString entryToCheck = QStringLiteral("cpuid.h");

the lambda is there to store the result in the `static const` value that will be returned. we don't want to compute the result multiple times

> clanghelpers.cpp:387
>          clangDebug() << "Trying" << dir;
> -        if (QFileInfo(dir).isDir()) {
> +        if (QFileInfo(dir).isDir()
> +            && QFile::exists(dir + QLatin1Char('/') + entryToCheck)) {

the `QFileInfo(dir).isDir()` can be removed, since the file check in the line below implicitly checks that too

> clanghelpers.cpp:397
> +        // chdir through with the updated version directory.
> +        auto hardcodedDir = QDir(QStringLiteral(KDEV_CLANG_BUILTIN_DIR));
> +        if (hardcodedDir.cd(QStringLiteral("../../%1/include").arg(clangVersion()))

actually, thinking about this once more, it can be further simplified similar to the path above:

  dir = QStringLiteral(KDEV_CLANG_BUILTIN_DIR "/../../%1/include").arg(clangVersion());
  if (QFile::exists(dir + QLatin1Char('/') + entryToCheck)) {
      return dir;

then all code paths are basically the same in regard to the `entryToCheck` check, just for different base paths. This could thus be shared, too:

  auto isValidDir = [](const QString &dir) {
      return QFile::exists(dir + QLatin1String("/cpuid.h"));

then use this in the three conditionals

  R32 KDevelop


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/d7668776/attachment-0001.html>

More information about the KDevelop-devel mailing list