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

[Ilya Biryukov via clangd-dev]

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
)? <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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/clangd-dev/attachments/20190604/f4badd06/attachment.html>