[PATCH] D30858: Teach llvm's path library to support both windows and posix paths at the same time.

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 13 03:29:01 PDT 2017


labath added a comment.

Lldb has a copy of a part of this code with the same type of `#ifdef` -> `if` changes applied, so it would be great it we could get rid of that.

What concerns me about this from a usability point of view is that the path style is not stored explicitly anywhere -- it just floats around implicitly. For working with remote paths, you're going to need some sort of an object which encapsulates this information. I suppose this could live in lldb's FileSpec as it did until now, but I'm wondering whether we shouldn't add something like that here, if we're going to start advertising foreign path support. I am asking that now because that would affect the signature of all of these functions, and it's probably better to avoid changing them twice.



================
Comment at: llvm/unittests/Support/Path.cpp:62
 #else
-  EXPECT_FALSE(path::is_separator('\\'));
+  EXPECT_FALSE(path::is_separator('\\', ));
 #endif
----------------
stray comma


https://reviews.llvm.org/D30858





More information about the llvm-commits mailing list