Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang-tools-extra/clangd/test/document-link.test | ||
---|---|---|
1 | nit: i'd drop "to reach resource_dir" to get this to fit on one line |
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?
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.
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\cSo, 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.
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)