clang: Also detect Clang builtin dirs at runtime
ClosedPublic

Authored by kfunk on Oct 4 2018, 10:07 PM.

Details

Summary

Changes:

  • Add a helper function retrieving the Clang version at runtime
  • Auto-detect bundled copy of Clang builtin headers on Windows
  • Print lots more debug output during startup

TODO: Wondering if we want to add more code detecting this directory at
runtime on other platforms, too. But for now, only Windows is
interesting for us.

CCBUG: 393779

Diff Detail

Repository
R32 KDevelop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kfunk created this revision.Oct 4 2018, 10:07 PM
Restricted Application added a project: KDevelop. · View Herald TranscriptOct 4 2018, 10:07 PM
Restricted Application added a subscriber: kdevelop-devel. · View Herald Transcript
kfunk requested review of this revision.Oct 4 2018, 10:07 PM
kfunk added a comment.Oct 4 2018, 10:11 PM

Note: Doing the retrieval at runtime would also allow distro packages to upgrade the libclang version under-the-hood; without having the need to recompile KDevelop. I.e. going from Clang/LLVM 6.0.0 to 6.0.1 (and keeping ABI), KDevelop would keep working just fine (which it currently wouldn't...).

kfunk added a comment.Oct 4 2018, 10:13 PM

Otoh, Clang 7 "fixes" this since they no longer use "X.Y.Z/include" but just "X/include" -- so we do not have to worry about this any longer anyway.

Don't we know the exact version of Clang at compile-time when building the bundled version? I think this change is mainly interesting for the "unbundled" case when Clang can be updated independently. Or what am I missing?

kfunk added a comment.Oct 5 2018, 6:45 AM

You're right. For the Windows case, we wouldn't actually need to query the Clang version at runtime, since we control the version of Clang/LLVM ourselves.

So in theory we could throw clangVersion() away, but I think it's useful to have in any way; even if it's just for printing some debug output during KDevelop startup (on Linux).

kossebau added inline comments.Oct 5 2018, 11:11 AM
plugins/clang/duchain/clanghelpers.cpp
375–387

Always confused about those, but should this not be QString::fromLocal8Bit, given it's consuming a raw string from the system?
Not that there might be actually and non-UTF8 around these days, but given the method exists, let's use it.

brauch accepted this revision.Oct 5 2018, 11:14 AM
brauch added a subscriber: brauch.

Looks good to me, and with little risk ... on Linux nothing changes, on Windows we test it anyways before shipping.

This revision is now accepted and ready to land.Oct 5 2018, 11:14 AM

Confirmed to work on openSUSE TW:

kdevelop.plugins.clang: Full Clang version: "clang version 6.0.1 (tags/RELEASE_601/final 335528)"
kdevelop.plugins.clang: Detected Clang version: "6.0.1"
kdevelop.plugins.clang: Using builtin dir: /usr/lib64/clang/6.0.1/include
kdevelop.plugins.clang: Using Clang builtin include path: "/usr/lib64/clang/6.0.1/include"

or when being nasty setting KDEV_CLANG_BUILTIN_DIR=/does/not/exist:

kdevelop.plugins.clang: Full Clang version: "clang version 6.0.1 (tags/RELEASE_601/final 335528)"
kdevelop.plugins.clang: Detected Clang version: "6.0.1"
kdevelop.plugins.clang: Using dir from $KDEV_CLANG_BUILTIN_DIR: "/does/not/exist"
kdevelop.plugins.clang: Using Clang builtin include path: "/does/not/exist"

which results in unloading the clang plugin, as expected.

plugins/clang/clangsupport.cpp
184

This debug statement duplicates a bit those of ClangHelpers::clangBuiltinIncludePath(), could perhaps be removed again?

kfunk marked an inline comment as done.Oct 5 2018, 12:38 PM
This revision was automatically updated to reflect the committed changes.