[Lldb-commits] [PATCH] D48084: [FileSpec] Delegate common operations to llvm::sys::path

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked an inline comment as done.
JDevlieghere added inline comments.


================
Comment at: source/Utility/FileSpec.cpp:244-246
+  // Only update style if explicitly requested.
+  if (style)
+    m_style = (*style == Style::native) ? GetNativeStyle() : *style;
----------------
labath wrote:
> I don't really have a clear opinion on whether the new behaviour here is better or not, but it any case this sounds like something worth explicitly calling out.
As far as the FileSpec class is concerned this doesn't change anything, we were always explicitly passing the m_style argument which is now implicit. To me this seems strictly better than what is was, were you might accidentally end up with the host path. 

I'll add a comment in the header.


================
Comment at: unittests/Utility/FileSpecTest.cpp:284
     "//",
-    "//a",
     "//a/",
----------------
labath wrote:
> Any particular reason for removing this?
Yup, llvm doesn't consider `C:` or `//net` to be absolute paths (as opposed to `C:\` or `//net/`. While this is debatable, it makes things consistent with drive names on Windows, which is why I added the net tests above. 


Repository:
  rL LLVM

https://reviews.llvm.org/D48084