xavierd / clang_complete

Vim plugin that use clang for completing C/C++ code.
http://www.vim.org/scripts/script.php?script_id=3302
1.96k stars 308 forks source link

Fixed libclang not finding builtin includes #490

Closed dnery closed 8 years ago

dnery commented 8 years ago

The original script considered that the list containing the subdirectories of the clang lib directory, when ordered, would have the entry corresponding to the current version of clang (which is the path it wants) in its first position, when it should actually be the last. Error would insue and the following would be output:

WARNING: libclang can not find the builtin includes. This will cause slow code completion. Please report the problem.

In the presence of other subdirectories or binaries (which is the case when a static analyzer is also present), subDir would contain the wrong result. The solution to this, implemented in this patch, is to, after sorting, also reverse the ordering of files, before defining subDir.

deffi420 commented 8 years ago

I think this fix is wrong. The PR indeed fixes the issue in some systems, mine included, but the approach is incorrect. We want to return the last, not first, subdirectory returned by os.listdir() because it is the builtin directory of the latest clang version. Suppose we have clang versions 3.7.0 and 3.8.0 installed in the same system, then your code returns the old 3.7.0 directory. Moreover, your code lacks os.path.isdir() check which caused the problem in the first place (note that os.listdir() returns files in addition to subdirectories!)

My fix proposal: PR #491

xaizek commented 8 years ago

I agree with @deffi420, couldn't understand why description here says first position, when it should actually be the last when current code already uses the last item.

@dnery, please give #491 a try (there is only 3.8.0 directory on my system, can fake new files, but better to have proper confirmation).

dnery commented 8 years ago

@deffi420 you're right, I was relying on the fact that clang cleans up the library directory upon updating. #491 works for me, is more precise and performs better... @xaizek ah, that was just bad wording on my part... I meant to say the script considers the wanted position being in last, but in reality it is the first, and for that it's broken. I also don't have any other version-correspondent subdirs in my clang lib dir, but I do have other binaries (static code analyzers, namely) and it works in such scenario. Closing this now.