[Lldb-commits] [PATCH] D19060: FileSpec: make matching separator-agnostic again

Zachary Turner via lldb-commits lldb-commits at lists.llvm.org
Wed Apr 13 08:31:19 PDT 2016


The expected behavior imo is to normalize before returning in
GetDirectory(). Perhaps you could add a boolean with default value true?
On Wed, Apr 13, 2016 at 8:26 AM Pavel Labath <labath at google.com> wrote:

> labath created this revision.
> labath added a reviewer: zturner.
> labath added a subscriber: lldb-commits.
>
> In D18689, I removed the call to Normalize() in FileSpec::SetFile, because
> it no longer seemed
> needed, and it resolved a quirk in the FileSpec API (spec.GetCString()
> returnes a path with
> backslashes, but spec.GetDirectory().GetCString() has forward slashes).
> This turned out to be a
> problem because we would consider paths with different separators as
> different (which led to
> unresolved breakpoints for instance).
>
> Here, I am putting back in the call to Normalize() and adding a unittest
> for FileSpec::Equal. I
> am commenting out the GetDirectory unittests until we figure out the what
> is the expected
> behaviour here.
>
> http://reviews.llvm.org/D19060
>
> Files:
>   source/Host/common/FileSpec.cpp
>   unittests/Host/FileSpecTest.cpp
>
> Index: unittests/Host/FileSpecTest.cpp
> ===================================================================
> --- unittests/Host/FileSpecTest.cpp
> +++ unittests/Host/FileSpecTest.cpp
> @@ -22,7 +22,7 @@
>
>      FileSpec fs_windows("F:\\bar", false, FileSpec::ePathSyntaxWindows);
>      EXPECT_STREQ("F:\\bar", fs_windows.GetCString());
> -    EXPECT_STREQ("F:\\", fs_windows.GetDirectory().GetCString());
> +    // EXPECT_STREQ("F:\\", fs_windows.GetDirectory().GetCString()); //
> It returns "F:/"
>      EXPECT_STREQ("bar", fs_windows.GetFilename().GetCString());
>
>      FileSpec fs_posix_root("/", false, FileSpec::ePathSyntaxPosix);
> @@ -38,16 +38,16 @@
>      FileSpec fs_windows_root("F:\\", false, FileSpec::ePathSyntaxWindows);
>      EXPECT_STREQ("F:\\", fs_windows_root.GetCString());
>      EXPECT_STREQ("F:", fs_windows_root.GetDirectory().GetCString());
> -    EXPECT_STREQ("\\", fs_windows_root.GetFilename().GetCString());
> +    // EXPECT_STREQ("\\", fs_windows_root.GetFilename().GetCString()); //
> It returns "/"
>
>      FileSpec fs_posix_long("/foo/bar/baz", false,
> FileSpec::ePathSyntaxPosix);
>      EXPECT_STREQ("/foo/bar/baz", fs_posix_long.GetCString());
>      EXPECT_STREQ("/foo/bar", fs_posix_long.GetDirectory().GetCString());
>      EXPECT_STREQ("baz", fs_posix_long.GetFilename().GetCString());
>
>      FileSpec fs_windows_long("F:\\bar\\baz", false,
> FileSpec::ePathSyntaxWindows);
>      EXPECT_STREQ("F:\\bar\\baz", fs_windows_long.GetCString());
> -    EXPECT_STREQ("F:\\bar", fs_windows_long.GetDirectory().GetCString());
> +    // EXPECT_STREQ("F:\\bar",
> fs_windows_long.GetDirectory().GetCString()); // It returns "F:/bar"
>      EXPECT_STREQ("baz", fs_windows_long.GetFilename().GetCString());
>
>      FileSpec fs_posix_trailing_slash("/foo/bar/", false,
> FileSpec::ePathSyntaxPosix);
> @@ -57,7 +57,7 @@
>
>      FileSpec fs_windows_trailing_slash("F:\\bar\\", false,
> FileSpec::ePathSyntaxWindows);
>      EXPECT_STREQ("F:\\bar\\.", fs_windows_trailing_slash.GetCString());
> -    EXPECT_STREQ("F:\\bar",
> fs_windows_trailing_slash.GetDirectory().GetCString());
> +    // EXPECT_STREQ("F:\\bar",
> fs_windows_trailing_slash.GetDirectory().GetCString()); // It returns
> "F:/bar"
>      EXPECT_STREQ(".",
> fs_windows_trailing_slash.GetFilename().GetCString());
>  }
>
> @@ -72,7 +72,7 @@
>      FileSpec fs_windows("F:\\bar", false, FileSpec::ePathSyntaxWindows);
>      fs_windows.AppendPathComponent("baz");
>      EXPECT_STREQ("F:\\bar\\baz", fs_windows.GetCString());
> -    EXPECT_STREQ("F:\\bar", fs_windows.GetDirectory().GetCString());
> +    // EXPECT_STREQ("F:\\bar", fs_windows.GetDirectory().GetCString());
> // It returns "F:/bar"
>      EXPECT_STREQ("baz", fs_windows.GetFilename().GetCString());
>
>      FileSpec fs_posix_root("/", false, FileSpec::ePathSyntaxPosix);
> @@ -84,7 +84,7 @@
>      FileSpec fs_windows_root("F:\\", false, FileSpec::ePathSyntaxWindows);
>      fs_windows_root.AppendPathComponent("bar");
>      EXPECT_STREQ("F:\\bar", fs_windows_root.GetCString());
> -    EXPECT_STREQ("F:\\", fs_windows_root.GetDirectory().GetCString());
> +    // EXPECT_STREQ("F:\\", fs_windows_root.GetDirectory().GetCString());
> // It returns "F:/"
>      EXPECT_STREQ("bar", fs_windows_root.GetFilename().GetCString());
>  }
>
> @@ -95,3 +95,17 @@
>      EXPECT_STREQ("/foo", fs.GetDirectory().GetCString());
>      EXPECT_STREQ("bar", fs.GetFilename().GetCString());
>  }
> +
> +TEST(FileSpecTest, Equal)
> +{
> +    FileSpec backward("C:\\foo\\bar", false,
> FileSpec::ePathSyntaxWindows);
> +    FileSpec forward("C:/foo/bar", false, FileSpec::ePathSyntaxWindows);
> +    EXPECT_EQ(forward, backward);
> +
> +    const bool full_match = true;
> +    const bool remove_backup_dots = true;
> +    EXPECT_TRUE(FileSpec::Equal(forward, backward, full_match,
> remove_backup_dots));
> +    EXPECT_TRUE(FileSpec::Equal(forward, backward, full_match,
> !remove_backup_dots));
> +    EXPECT_TRUE(FileSpec::Equal(forward, backward, !full_match,
> remove_backup_dots));
> +    EXPECT_TRUE(FileSpec::Equal(forward, backward, !full_match,
> !remove_backup_dots));
> +}
> Index: source/Host/common/FileSpec.cpp
> ===================================================================
> --- source/Host/common/FileSpec.cpp
> +++ source/Host/common/FileSpec.cpp
> @@ -394,6 +394,8 @@
>          m_is_resolved = true;
>      }
>
> +    Normalize(resolved, syntax);
> +
>      llvm::StringRef resolve_path_ref(resolved.c_str());
>      size_t dir_end = ParentPathEnd(resolve_path_ref, syntax);
>      if (dir_end == 0)
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160413/de2b585c/attachment.html>


More information about the lldb-commits mailing list