[llvm][Support] Delete unused is_absolute_gnu helper

[certl]

The other path manipulation functions have similar quirks for Windows paths (Unicode drives / NTFS alternate streams). I'm not particularly concerned, but for correctness sake I would also recommend for those to be removed / refactored / replaced with something standard such as those from C++17.

find_first_component - assumes all drives are single byte alpha characters (see here for how the git project handles this):

if (real_style(style) == Style::windows) {
  // C:
  if (path.size() >= 2 &&
      std::isalpha(static_cast<unsigned char>(path[0])) && path[1] == ':')
    return path.substr(0, 2);
}

root_dir_start - also assumes single byte drive:

// case "c:/"
if (real_style(style) == Style::windows) {
  if (str.size() > 2 && str[1] == ':' && is_separator(str[2], style))
    return 2;
}

filename_pos - doesn't anticipate NTFS alternate streams (eg: I believe C:\xxx\yyy::$INDEX_ALLOCATION\ would return the filename as $INDEX_ALLOCATION), as well as assuming single byte drives:

if (real_style(style) == Style::windows) {
  if (pos == StringRef::npos)
    pos = str.find_last_of(':', str.size() - 2);
}

[kadircet]

Hi @tinti this code seems to be recently introduced without any usage, and as pointed out by @certl it doesn't consider multi-byte drive letters.

Are you planning to add any usages to the helper? If so, please consider Christopher's points about potential problems.
If you are not planning to add any usages, I beleive it would be best to delete the helper to reduce confusion.

WDYT?

[tinti]

@

In D88502#2303036, @kadircet wrote:

Hi @tinti this code seems to be recently introduced without any usage, and as pointed out by @certl it doesn't consider multi-byte drive letters.

Hi @certl and @kadircet.

It intentionally does not consider multi-byte drive letters because it mimics GNU's objdump (which does not either).

Are you planning to add any usages to the helper? If so, please consider Christopher's points about potential problems.
If you are not planning to add any usages, I beleive it would be best to delete the helper to reduce confusion.

Yes I am. I am about to submit another patch for llvm-objdump.

WDYT?

We considered using LLVM's helpers but would be too much work. It is simpler to do this way.

Please take a look at: https://reviews.llvm.org/D87667

llvm/include/llvm/Support/Path.h

 ///
 /// \see has_root_name
 /// \see has_root_directory
 ///
 /// @param path Input path.
 /// @result True if the path is absolute, false if it is not.
 bool is_absolute(const Twine &path, Style style = Style::native);
 
-/// Is path absolute using GNU rules?
-///
-/// GNU rules are:
-/// 1) Paths starting with a path separator are absolute.
-/// 2) Windows style paths are also absolute if they start with a character
-///    followed by ':'.
-/// 3) No other paths are absolute.
-///
-/// On Windows style the path "C:\Users\Default" has "C:" as root name and "\"
-/// as root directory.
-///
-/// Hence "C:" on Windows is absolute under GNU rules and not absolute under
-/// C++17 because it has no root directory. Likewise "/" and "\" on Windows are
-/// absolute under GNU and are not absolute under C++17 due to empty root name.
-///
-/// \see has_root_name
-/// \see has_root_directory
-///
-/// @param path Input path.
-/// @param style The style of \p path (e.g. Windows or POSIX). "native" style
-/// means to derive the style from the host.
-/// @result True if the path is absolute following GNU rules, false if it is
-/// not.
-bool is_absolute_gnu(const Twine &path, Style style = Style::native);
-
 /// Is path relative?
 ///
 /// @param path Input path.
 /// @result True if the path is relative, false if it is not.
 bool is_relative(const Twine &path, Style style = Style::native);
 
 /// Remove redundant leading "./" pieces and consecutive separators.
 ///

llvm/lib/Support/Path.cpp

bool is_absolute(const Twine &path, Style style) {
 
   bool rootDir = has_root_directory(p, style);
   bool rootName =
       (real_style(style) != Style::windows) || has_root_name(p, style);
 
   return rootDir && rootName;
 }
 
-bool is_absolute_gnu(const Twine &path, Style style) {
-  SmallString<128> path_storage;
-  StringRef p = path.toStringRef(path_storage);
-
-  // Handle '/' which is absolute for both Windows and POSIX systems.
-  // Handle '\\' on Windows.
-  if (!p.empty() && is_separator(p.front(), style))
-    return true;
-
-  if (real_style(style) == Style::windows) {
-    // Handle drive letter pattern (a character followed by ':') on Windows.
-    if (p.size() >= 2 && (p[0] && p[1] == ':'))
-      return true;
-  }
-
-  return false;
-}
-
 bool is_relative(const Twine &path, Style style) {
   return !is_absolute(path, style);
 }
 
 StringRef remove_leading_dotslash(StringRef Path, Style style) {
   // Remove leading "./" (or ".//" or "././" etc.)
   while (Path.size() > 2 && Path[0] == '.' && is_separator(Path[1], style)) {
     Path = Path.substr(2);

llvm/unittests/Support/Path.cpp

 
 #ifdef _WIN32
   EXPECT_TRUE(path::is_separator('\\'));
 #else
   EXPECT_FALSE(path::is_separator('\\'));
 #endif
 }
 
-TEST(is_absolute_gnu, Works) {
-  // Test tuple <Path, ExpectedPosixValue, ExpectedWindowsValue>.
-  const std::tuple<StringRef, bool, bool> Paths[] = {
-      std::make_tuple("", false, false),
-      std::make_tuple("/", true, true),
-      std::make_tuple("/foo", true, true),
-      std::make_tuple("\\", false, true),
-      std::make_tuple("\\foo", false, true),
-      std::make_tuple("foo", false, false),
-      std::make_tuple("c", false, false),
-      std::make_tuple("c:", false, true),
-      std::make_tuple("c:\\", false, true),
-      std::make_tuple("!:", false, true),
-      std::make_tuple("xx:", false, false),
-      std::make_tuple("c:abc\\", false, true),
-      std::make_tuple(":", false, false)};
-
-  for (const auto &Path : Paths) {
-    EXPECT_EQ(path::is_absolute_gnu(std::get<0>(Path), path::Style::posix),
-              std::get<1>(Path));
-    EXPECT_EQ(path::is_absolute_gnu(std::get<0>(Path), path::Style::windows),
-              std::get<2>(Path));
-  }
-}
-
 TEST(Support, Path) {
   SmallVector<StringRef, 40> paths;
   paths.push_back("");
   paths.push_back(".");
   paths.push_back("..");
   paths.push_back("foo");
   paths.push_back("/");
   paths.push_back("/foo");
for (SmallVector<StringRef, 40>::const_iterator i = paths.begin(),
     (void)path::parent_path(*i);
     (void)path::has_filename(*i);
     (void)path::filename(*i);
     (void)path::has_stem(*i);
     (void)path::stem(*i);
     (void)path::has_extension(*i);
     (void)path::extension(*i);
     (void)path::is_absolute(*i);
-    (void)path::is_absolute_gnu(*i);
     (void)path::is_relative(*i);
 
     SmallString<128> temp_store;
     temp_store = *i;
     ASSERT_NO_ERROR(fs::make_absolute(temp_store));
     temp_store = *i;
     path::remove_filename(temp_store);