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

Tue Jun 4 10:16:38 PDT 2019

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/20190604/3ad47908/attachment.html>