search

Sarcasm / irony-mode

A C/C++ minor mode for Emacs powered by libclang
GNU General Public License v3.0
901 stars 98 forks source link

Search every subdir under /usr/lib{,64}/clang #529

Closed tastytea closed 5 years ago

tastytea commented 5 years ago

LIBCLANG_KNOWN_LLVM_VERSIONS in FindLibClang.cmake will never contain all possible versions. Using a wildcard is simpler.

Sarcasm commented 5 years ago

Hello, thanks for the PR, however I'm not convinced by this fix.

I would go another route. Recent versions of clang, should distribute an ClangConfig.cmake, which create the libclang IMPORTED target. And the usual CMake arguments can be used to point to a specific version if needed. However, when I tried to use this method, I found some issues, which I tried to fix: https://reviews.llvm.org/D30911 The initial discussion which shows what find_package(Clang) returns: https://github.com/Sarcasm/irony-mode/pull/368/files I think the fix is in since Clang >= 5.1, but I worked on my patch since then.

So maybe you can take a look at using find_package(Clang), or I should try that myself, still have the git stash around.

Also a few questions:

tastytea commented 5 years ago

I could not find documentation that states that find_path( PATHS ) supports globbing, but it works.

list( APPEND ) has also no documented support for globbing. It does support globbing in cmake 3.9.6 on Gentoo but apparently not in cmake 3.9.2 on Ubuntu.

I would guess that my method picks the first (oldest) version that it finds.

In summary, I don't think my patch is a good idea anymore.

Sarcasm commented 5 years ago

Ok, I will close the issue then But thank you for bringing this up.

I made a patch to use find_package(Clang):

The old code is still used as a fallback. If the patch happens to work (I have some doubts), it will not be necessary to update the list of versions anymore.