[PATCH] D111457: [clang][test] Add lit helper for windows paths

Keith Smiley via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 28 15:49:09 PST 2022


keith added inline comments.


================
Comment at: clang/test/lit.cfg.py:60
+if platform.system() == 'Windows':
+    root_sep = 'C:\\'
+else:
----------------
compnerd wrote:
> This isn't really a separator, this is the root itself.  I think that it makes sense to name it accordingly.  Please do not hard code this to `C:\`.  There is no guarantee that the root is `C:\`.
Yea I didn't like the original naming here either, any suggestions? I avoid `path` in these because of the existing `pathsep`, maybe `fsroot` and `fssep`?

What are you suggesting in place of hardcoding `C:\`? I don't think we could detect what folks are using in the tests, so it does seem like it would be up for individual tests to do this however it makes sense. It appears that there are a few hundred uses of this in tests today, does that break in general if you're using another drive?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111457



More information about the cfe-commits mailing list