[clangd][lit] Update document-link.test to respect custom resource-dir locations

[sammccall]

clang-tools-extra/clangd/test/document-link.test

1–2

nit: i'd drop "to reach resource_dir" to get this to fit on one line
(Remembering that this quirk is almost always uninteresting to people reading this test)

[kadircet]

• update comment

[ArcsinX]

Just a question.
On Windows %resource_dir/.. looks a bit inconsistent after substitution (d:\llvm-project\build\lib\clang\12.0.0\include/..), but this test passes.
As far as Windows accepts forward and back slashes, I expect something like this %/resource_dir/.. to be consistent across all platforms.
Are there any plans to add %/resource_dir substitution as it was done for %T, %s, etc?

[kadircet]

not that i know of.

[sammccall]

As far as Windows accepts forward and back slashes

Note we don't rely on windows itself for this support.
All access through llvm::sys::fs APIs on windows ultimately goes through widenPath to convert to UTF-16, and this substitutes slashes.
So LLVM tools do always support / on windows and it's fairly common to rely on this for tests (abstraction is hard in lit tests).

[ArcsinX]

In D88721#2308296, @sammccall wrote:

As far as Windows accepts forward and back slashes

Note we don't rely on windows itself for this support.
All access through llvm::sys::fs APIs on windows ultimately goes through widenPath to convert to UTF-16, and this substitutes slashes.
So LLVM tools do always support / on windows and it's fairly common to rely on this for tests (abstraction is hard in lit tests).

I am not sure that understood you correctly, but widenPath does nothing for me, if I pass mixed-slash path to it.
Code:

std::string From("C:\\a/b\\c");
SmallVector<wchar_t, 128> To;
llvm::sys::windows::widenPath(From, To);
std::wcerr << To.data();

Output:

C:\a/b\c

So, if we imagine that Windows does not support /, then paths with / could not be opened with llvm::sys::fs API.

[sammccall]

In D88721#2308361, @ArcsinX wrote:

In D88721#2308296, @sammccall wrote:

As far as Windows accepts forward and back slashes

Note we don't rely on windows itself for this support.
All access through llvm::sys::fs APIs on windows ultimately goes through widenPath to convert to UTF-16, and this substitutes slashes.
So LLVM tools do always support / on windows and it's fairly common to rely on this for tests (abstraction is hard in lit tests).

I am not sure that understood you correctly, but widenPath does nothing for me, if I pass mixed-slash path to it.

Ack, you're right. I've looked at this before, and was looking at this line:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/Windows/Path.inc#L103
But in the common case, we've already exited by then, so you're absolutely right...

Code:

std::string From("C:\\a/b\\c");
SmallVector<wchar_t, 128> To;
llvm::sys::windows::widenPath(From, To);
std::wcerr << To.data();

Output:

C:\a/b\c

So, if we imagine that Windows does not support /, then paths with / could not be opened with llvm::sys::fs API.

But at a higher level, ISTM the purpose of widenPath is exactly to produce a path that the win32 APIs will accept, including if the input has / (as shown by the long-path case I was confused by).
So the contract of the llvm functions ensuring that forward slashes are safe still seems to be there.

clang-tools-extra/clangd/test/document-link.test

-# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+# %resource_dir actually points at builtin_include_dir, go up one directory.
+# RUN: clangd -lit-test -resource-dir=%resource_dir/.. < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"#include <stdint.h>\n#include <stddef.h>"}}}
 ---
 {"jsonrpc":"2.0","id":2,"method":"textDocument/documentLink","params":{"textDocument":{"uri":"test:///main.cpp"}}}
 #      CHECK:  "id": 2,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [