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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D88721#2308361 <https://reviews.llvm.org/D88721#2308361>, @ArcsinX wrote:

> In D88721#2308296 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88721/new/

https://reviews.llvm.org/D88721