[clangd-dev] Automatically adding includes always inserts the full path on windows

Thu Jun 6 09:34:26 PDT 2019

Hi Stefan,

I've sent out https://reviews.llvm.org/D62965 let me know if it works for
you.

On Tue, Jun 4, 2019 at 7:16 PM Ilya Biryukov <ibiryukov at google.com> wrote:

> Thanks, I'll try to repro and get back to you with the results of my
> investigation.
> Hacking JSONCompilationDatabase also looks like an overkill if it's just
> clangd that is affected. However, if it turns out more tools break because
> of this, might actually be the right fix.
>
> On Tue, Jun 4, 2019 at 7:03 PM Stefan Lietzau <s.lietzau at tesis.de> wrote:
>
>> I think a simple CMake project with one source file and header should
>> work. Maybe it’s required that the header directory is a relative path
>> containing ..\
>>
>> I’m not sure if CMake always generates forward slashes for the directory
>> part of a compile command but that’s very important to reproduce the issue.
>>
>>
>>
>> Locally I was able to “fix” the issue by hacking the
>> JSONCompilationDatabase to return the directory of the compile command as a
>> native path instead of returning the unprocessed JSON string.
>>
>> However I’m not sure if that’s the solution one would want to go for as
>> that change would affect a lot of other parts of the codebase.
>>
>>
>>
>>
>>
>> *Von:* Ilya Biryukov [mailto:ibiryukov at google.com]
>> *Gesendet:* Dienstag, 4. Juni 2019 17:50
>> *An:* Stefan Lietzau
>> *Cc:* Kadir Çetinkaya; via clangd-dev
>> *Betreff:* Re: [clangd-dev] Automatically adding includes always inserts
>> the full path on windows
>>
>>
>>
>> Ok, that means we'll have to take a closer look.
>>
>> I can try reproducing on Windows, any guidance on how to get a minimal
>> repro? E.g. would it work on a simple hello-world example, e.g. for
>> including standard library symbols?
>>
>>
>>
>> On Tue, Jun 4, 2019 at 4:47 PM Stefan Lietzau <s.lietzau at tesis.de> wrote:
>>
>> I just tested the issue again and the problem is not fixed with a master
>> version of today(commit 2e49e8196dab68481857ec243f091fbb4ad7af43).
>>
>>
>>
>> Sorry for the confusion, I just checked and I’m already using
>> https://github.com/llvm/llvm-project.git
>>
>>
>>
>> Regards,
>>
>> Stefan Lietzau
>>
>>
>>
>>
>>
>> *Von:* Ilya Biryukov [mailto:ibiryukov at google.com]
>> *Gesendet:* Dienstag, 4. Juni 2019 15:15
>> *An:* Stefan Lietzau
>> *Cc:* Kadir Çetinkaya; via clangd-dev
>> *Betreff:* Re: [clangd-dev] Automatically adding includes always inserts
>> the full path on windows
>>
>>
>>
>> Ok, so Kadir's patch will very likely fix the problem. Please let us know
>> if the issue persists.
>>
>>
>>
>> Offtopic question: any reason (other than historical) why you're using
>> the mirror instead of the official monorepo(
>> https://github.com/llvm/llvm-project)?
>>
>>
>>
>> On Tue, Jun 4, 2019 at 2:15 PM Stefan Lietzau <s.lietzau at tesis.de> wrote:
>>
>> Hey Ilya,
>>
>>
>>
>> I was using commit ed4dbe63260f5a0a3410995b85b32f0ec34b0076 from the
>> github llvm-mirror, dated 14 May 2019.
>>
>>
>>
>> During my debugging I noticed that this function (
>> https://github.com/llvm-mirror/clang/blob/b84e04d4c031c8ff9a1a6875381f4a34b923cad7/lib/Lex/HeaderSearch.cpp#L1677
>> ) tried to compare paths similar to these:
>>
>> C:\foo/bar/baz with C:/foo/bar/baz. As these are compared as strings,
>> they don’t match. I traced the problem back until I found that the
>> compilation database entry contains only forward slashes while the
>> suggested header contained backslashes. I’m not sure what the policy for
>> path handling is for clang tooling and clangd in general. Without such
>> information it’s difficult to fix this bug the right way.
>>
>>
>>
>> @Kadir Çetinkaya <kadircet at google.com>, to me it looks like your change
>> should solve the issue, I’ll test it and report my findings later.
>> Comparing the date from the commit I used to build locally with the date of
>> your change, suggests to me that my build should have included your fix.
>>
>>
>>
>> Kind regards,
>>
>>
>>
>> *Stefan Lietzau*
>>
>>
>>
>> *Von:* Ilya Biryukov [mailto:ibiryukov at google.com]
>> *Gesendet:* Dienstag, 4. Juni 2019 09:29
>> *An:* Stefan Lietzau; Kadir Çetinkaya
>> *Cc:* via clangd-dev
>> *Betreff:* Re: [clangd-dev] Automatically adding includes always inserts
>> the full path on windows
>>
>>
>>
>> Hi Stefan,
>>
>>
>>
>> Thanks for reporting this. I don't think any of the active clangd
>> developers run on Windows, so bugs like this can easily go unnoticed.
>>
>> To add to what Kadir was requesting, could you point to a particular
>> point in code that does the string comparison? If some of the strings come
>> from user input (e.g. flags to the compiler), we should definitely
>> normalize them before comparisons.
>>
>>
>>
>> @Kadir Çetinkaya <kadircet at google.com>, does your change also cover
>> non-diagnostic use-cases? The function name looks very diagnostic-specific
>>
>>
>>
>> On Mon, Jun 3, 2019 at 8:20 PM Stefan Lietzau via clangd-dev <
>> clangd-dev at lists.llvm.org> wrote:
>>
>> Hi there,
>>
>>
>>
>> I’m using clangd with vscode on windows with the option to add includes
>> when autocompleting symbols. However the inserted include is always
>> absolute, even if the file is present in the include path
>> (compile-commands.json generated by CMake). I tracked the problem down to a
>> path comparison where one path coming from the compilation database
>> contains forward slashes and the other path (header where the symbol
>> definition was found) contains backslashes. Since this is a simple string
>> comparison it fails and determines the header is not part of the include
>> directories.
>>
>>
>>
>> I don’t think the best fix would be to convert the paths before
>> comparison or use a better comparison. Instead I think this is something
>> that needs to be fixed deep inside clang tooling. That means I think the
>> paths returned by the compilation database should be in a predictable
>> format.
>>
>>
>>
>> What do you guys think?
>>
>>
>>
>> Kind regards
>>
>>
>>
>> *Stefan*
>>
>>
>>
>> _______________________________________________
>> clangd-dev mailing list
>> clangd-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/clangd-dev
>>
>>
>>
>>
>> --
>>
>> Regards,
>>
>> Ilya Biryukov
>>
>>
>>
>>
>> --
>>
>> Regards,
>>
>> Ilya Biryukov
>>
>>
>>
>>
>> --
>>
>> Regards,
>>
>> Ilya Biryukov
>>
>
>
> --
> Regards,
> Ilya Biryukov
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/clangd-dev/attachments/20190606/a90ec815/attachment.html>