[PATCH] D45942: [Support/Path] Make handling of paths like "///" consistent

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: unittests/Support/Path.cpp:186-190
+  EXPECT_EQ("/", path::filename("//"));
+  EXPECT_EQ("", path::parent_path("//"));
+
+  EXPECT_EQ("/", path::filename("///"));
+  EXPECT_EQ("", path::parent_path("///"));
----------------
zturner wrote:
> A comment here maybe in order.  Should this behavior be true for both path styles?
I am not sure which aspect of this do you want to me comment on, as there several potentially controversial things going on here:
- decomposition of `//`: posix states that paths beginning with `//` are magic (I'm not sure if windows has a spec, but they are magic there as well), so treating it the same way as "/" is not entirely correct. It might be better to decompose this to `""+"//"`, but that is not completely consistent either, as elsewhere we treat "//net" as a single component, not as `//` followed by "net". (this holds for both path styles)
- treatment of `\\\` on windows. It seems windows just refuses to resolve paths like this (though it is fine with multiple backslashes elsewhere in the path), so treating this the same way as `\` is not entirely correct. However, I think it is somewhat better than treating it as ""+".", which we were doing before. (this is a windows speciality as posix will happily accept paths like this).

In general, I'm not trying to be overly-prescriptive about the *right* behavior here. I am just trying to make sure we do something vaguely reasonable, and I am totally open to changing there is a need for it, now or in the future. (Maybe I should put that in the comment?) It's just that right now this was the easiest to implement.


Repository:
  rL LLVM

https://reviews.llvm.org/D45942