[clang][HeaderSuggestion] Handle the case of dotdot with an absolute path

[kadircet]

Summary

Include insertion in clangd was inserting absolute paths when the
include directory was an absolute path with a double dot. This patch makes sure
double dots are handled both with absolute and relative paths.

[sammccall]

Well done!
I never managed to track this one down, this was really annoying.

[ormris]

clang-tools-extra/unittests/clangd/HeadersTests.cpp

220 ↗

(On Diff #195751)

Quick nit: This test won't work on Windows as it only tests for POSIX-style path separators. You could easily add a Windows ifdef, though.

[sammccall]

clang-tools-extra/unittests/clangd/HeadersTests.cpp

220 ↗

(On Diff #195751)

testPath() returns an appropriate native path, and calculate() returns an #include string (which always uses /).
The "../" on line 217 looks a little more suspicious, but I think the clang driver might silently Do What I Mean here.
Are you seeing test failures?

[ormris]

clang-tools-extra/unittests/clangd/HeadersTests.cpp

220 ↗

(On Diff #195751)

Yes, here's the output.

[ RUN      ] HeadersTest.ShortenedInclude
C:\llvm\tools\clang\tools\extra\unittests\clangd\HeadersTests.cpp(220): error:       Expected: calculate(BarHeader)
      Which is: "\"sub\\bar.h\""
To be equal to: "\"sub/bar.h\""

[sammccall]

clang-tools-extra/unittests/clangd/HeadersTests.cpp

220 ↗

(On Diff #195751)

Hmm, that suggests the test is catching a real bug in the code: we should be inserting paths with forward slashes even on Windows.

Looking at the test cases, i have the sinking feeling we've always been doing this and never noticed as no test case has a slash in the final #include. (And none of the main clangd devs are primarily using Windows)

Thank you for pointing this out!

@kadircet: we should either fix this before this patch lands, or ifdef it until we can fix.

[kadircet]

Sent out D60995, hopefully it should fix the issue. Will wait until that lands.