mstorsjo added inline comments.
================ Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:546 fs::make_absolute(TestScheme::TestDir, Path); + path::native(Path); return std::string(Path); ---------------- mstorsjo wrote: > sammccall wrote: > > This change is an example of something subtle that I don't understand, and > > don't expect other maintainers to understand, which is therefore fragile. > Hmm, I'll have to revisit this particular change and see if it really was > necessary in the end, and add a comment explaining the subtlety. Ah, yes, now I remember what this particular change does: `TestScheme::TestDir` is a haredcoded constant string; `fs::make_absolute(TestScheme::TestDir, Path);` concatenates the always-backslashes `TestDir` with `Path` (with the currently preferred separator inbetween). By calling `native()` afterwards, it converts the whole resulting path to the native form. An alternative would be to provide `TestDir` in the preferred form (e.g. like wrapping it in a function that returns a variable string, like in TestFS.{h,cpp}.) - that'd maybe be more straightforward, but also feels a bit needless as we're calling `native()` on it anyway. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113268/new/ https://reviews.llvm.org/D113268 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits