Skip to content

Commit

Permalink
[Support/Path] sys::path::replace_path_prefix fix and simplifications
Browse files Browse the repository at this point in the history
Added unit tests for 2 scenarios that were failing.
Made replace_path_prefix back to 3 parameters instead of 5, simplifying the implementation. The other 2 were always used with the default value.

This commit is intended to be the first of 3:
1) simplify/fix replace_path_prefix.
2) use it in the context of -fdebug-prefix-map and -fmacro-prefix-map (see D76869).
3) Make Windows version of replace_path_prefix insensitive to both case and separators (slash vs backslash).

Differential Revision: https://reviews.llvm.org/D77223
  • Loading branch information
sylvain-audi committed Apr 3, 2020
1 parent aaaa25e commit e4ae0a2
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 61 deletions.
19 changes: 5 additions & 14 deletions llvm/include/llvm/Support/Path.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,32 +153,23 @@ void replace_extension(SmallVectorImpl<char> &path, const Twine &extension,
/// @code
/// /foo, /old, /new => /foo
/// /old, /old, /new => /new
/// /old, /old/, /new, false => /old
/// /old, /old/, /new, true => /new
/// /old, /old/, /new => /old
/// /old/foo, /old, /new => /new/foo
/// /old/foo, /old/, /new => /new/foo
/// /old/foo, /old/, /new/ => /new/foo
/// /oldfoo, /old, /new => /oldfoo
/// /foo, <empty>, /new => /new/foo
/// /foo, <empty>, new => new/foo
/// /old/foo, /old, <empty>, false => /foo
/// /old/foo, /old, <empty>, true => foo
/// /old/foo, /old, <empty> => /foo
/// @endcode
///
/// @param Path If \a Path starts with \a OldPrefix modify to instead
/// start with \a NewPrefix.
/// @param OldPrefix The path prefix to strip from \a Path. Any trailing
/// path separator is ignored if strict is true.
/// @param OldPrefix The path prefix to strip from \a Path.
/// @param NewPrefix The path prefix to replace \a NewPrefix with.
/// @param style The path separator style
/// @param strict If strict is true, a directory separator following
/// \a OldPrefix will also be stripped. Otherwise, directory
/// separators will only be matched and stripped when present
/// in \a OldPrefix.
/// @result true if \a Path begins with OldPrefix
bool replace_path_prefix(SmallVectorImpl<char> &Path,
const StringRef &OldPrefix, const StringRef &NewPrefix,
Style style = Style::native, bool strict = false);
bool replace_path_prefix(SmallVectorImpl<char> &Path, StringRef OldPrefix,
StringRef NewPrefix);

/// Append to path.
///
Expand Down
36 changes: 6 additions & 30 deletions llvm/lib/Support/Path.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -496,49 +496,25 @@ void replace_extension(SmallVectorImpl<char> &path, const Twine &extension,
path.append(ext.begin(), ext.end());
}

bool replace_path_prefix(SmallVectorImpl<char> &Path,
const StringRef &OldPrefix, const StringRef &NewPrefix,
Style style, bool strict) {
bool replace_path_prefix(SmallVectorImpl<char> &Path, StringRef OldPrefix,
StringRef NewPrefix) {
if (OldPrefix.empty() && NewPrefix.empty())
return false;

StringRef OrigPath(Path.begin(), Path.size());
StringRef OldPrefixDir;

if (!strict && OldPrefix.size() > OrigPath.size())
return false;

// Ensure OldPrefixDir does not have a trailing separator.
if (!OldPrefix.empty() && is_separator(OldPrefix.back()))
OldPrefixDir = parent_path(OldPrefix, style);
else
OldPrefixDir = OldPrefix;

if (!OrigPath.startswith(OldPrefixDir))
if (!OrigPath.startswith(OldPrefix))
return false;

if (OrigPath.size() > OldPrefixDir.size())
if (!is_separator(OrigPath[OldPrefixDir.size()], style) && strict)
return false;

// If prefixes have the same size we can simply copy the new one over.
if (OldPrefixDir.size() == NewPrefix.size() && !strict) {
if (OldPrefix.size() == NewPrefix.size()) {
llvm::copy(NewPrefix, Path.begin());
return true;
}

StringRef RelPath = OrigPath.substr(OldPrefixDir.size());
StringRef RelPath = OrigPath.substr(OldPrefix.size());
SmallString<256> NewPath;
path::append(NewPath, style, NewPrefix);
if (!RelPath.empty()) {
if (!is_separator(RelPath[0], style) || !strict)
path::append(NewPath, style, RelPath);
else
path::append(NewPath, style, relative_path(RelPath, style));
}

(Twine(NewPrefix) + RelPath).toVector(NewPath);
Path.swap(NewPath);

return true;
}

Expand Down
22 changes: 5 additions & 17 deletions llvm/unittests/Support/Path.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1254,21 +1254,14 @@ TEST(Support, ReplacePathPrefix) {
path::replace_path_prefix(Path, OldPrefix, EmptyPrefix);
EXPECT_EQ(Path, "/foo");
Path = Path2;
path::replace_path_prefix(Path, OldPrefix, EmptyPrefix, path::Style::native,
true);
path::replace_path_prefix(Path, OldPrefixSep, EmptyPrefix);
EXPECT_EQ(Path, "foo");
Path = Path3;
path::replace_path_prefix(Path, OldPrefix, NewPrefix, path::Style::native,
false);
path::replace_path_prefix(Path, OldPrefix, NewPrefix);
EXPECT_EQ(Path, "/newnew/foo");
Path = Path3;
path::replace_path_prefix(Path, OldPrefix, NewPrefix, path::Style::native,
true);
EXPECT_EQ(Path, "/oldnew/foo");
Path = Path3;
path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, path::Style::native,
true);
EXPECT_EQ(Path, "/oldnew/foo");
path::replace_path_prefix(Path, OldPrefix, NewPrefix2);
EXPECT_EQ(Path, "/longernewnew/foo");
Path = Path1;
path::replace_path_prefix(Path, EmptyPrefix, NewPrefix);
EXPECT_EQ(Path, "/new/foo");
Expand All @@ -1279,13 +1272,8 @@ TEST(Support, ReplacePathPrefix) {
path::replace_path_prefix(Path, OldPrefix, NewPrefix);
EXPECT_EQ(Path, "/new/");
Path = OldPrefix;
path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, path::Style::native,
false);
path::replace_path_prefix(Path, OldPrefixSep, NewPrefix);
EXPECT_EQ(Path, "/old");
Path = OldPrefix;
path::replace_path_prefix(Path, OldPrefixSep, NewPrefix, path::Style::native,
true);
EXPECT_EQ(Path, "/new");
}

TEST_F(FileSystemTest, OpenFileForRead) {
Expand Down

0 comments on commit e4ae0a2

Please sign in to comment.