[PATCH] D87667: [Support/Path] Add path::is_gnu_style_absolute

[Renato Golin via Phabricator via llvm-commits]

rengolin added inline comments.


================
Comment at: llvm/lib/Support/Path.cpp:704
+    // Handle drive letter pattern (i.e "c:") on Windows.
+    if (P.size() >= 2 && (P[0] && P[1] == ':'))
+      return true;
----------------
jhenderson wrote:
> tinti wrote:
> > rengolin wrote:
> > > Neither `7:` nor `$:` are valid paths on Windows, I think.
> > > 
> > > Perhaps `std::isalpha(P[0])` would help.
> > > Neither `7:` nor `$:` are valid paths on Windows, I think.
> > > 
> > > Perhaps `std::isalpha(P[0])` would help.
> > 
> > I thought it too but the code on GNU does not check `isalpha`.
> > 
> > ```
> > #define HAS_DRIVE_SPEC_1(dos_based, f)			\
> >   ((f)[0] && ((f)[1] == ':') && (dos_based))
> > ```
> > 
> > The Wikipedia also points an extension [1].
> > 
> > 
> > 
> > ```
> > If access to more filesystems than Z: is required under Windows NT, Volume Mount Points must be used.[11] However, it is possible to mount non-letter drives, such as 1:, 2:, or !: using the command line SUBST utility in Windows XP or later (i.e. SUBST 1: C:\TEMP), but it is not officially supported and may break programs that assume that all drives are letters A: to Z:.
> > ```
> > 
> > There is code on LLDB that does the way you suggested [2].
> > 
> > [1] https://en.wikipedia.org/wiki/Drive_letter_assignment#Common_assignments
> > [2] https://github.com/llvm/llvm-project/blob/bc9b6b33a0d5760370e72ae06c520c25aa8d61c6/lldb/source/Utility/FileSpec.cpp#L315
> We should allow what GNU allows, even if it doesn't make much sense. It's unlikely to matter that much, and the code is simpler this way.
@tinti: Why, Windows? Why?!

@jhenderson: good point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87667