[Lldb-commits] [PATCH] D48084: [FileSpec] Delegate common operations to llvm::sys::path
Jonas Devlieghere via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Tue Jun 12 12:46:51 PDT 2018
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;
> 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
> 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.
More information about the lldb-commits