FileSpec: make matching separator-agnostic again

[labath]

Summary

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.

[zturner]

The expected behavior imo is to normalize before returning in
GetDirectory(). Perhaps you could add a boolean with default value true?

[labath]

It's a bit more complicated than it seems. GetDirectory() returns a reference into the FileSpec, and a lot of code modifies it through it. Changing that would require fixups in all of the users, which I'd want to do separately.

But I do agree that this is the best way forward.

[zturner]

Ahh that's unfortunate. I guess the thing to do is make SetFile and
SetDirectory. At least by changing the return type we can get the compiler
to tell us everywhere this is happening. Did you run the test suite on
Windows to make sure this doesnt break anything?

[labath]

In D19060#399915, @zturner wrote:

Ahh that's unfortunate. I guess the thing to do is make SetFile and
SetDirectory. At least by changing the return type we can get the compiler
to tell us everywhere this is happening. Did you run the test suite on
Windows to make sure this doesnt break anything?

Yeah, I had something like that in mind.

I didn't run the windows test suite, but I'd be very surprised if it broke anything, since I'm basically just restoring the previous behavior (before I started messing with this function, we were always running the normalization).

[zturner]

Ok sounds good

lldb/trunk/source/Host/common/FileSpec.cpp

FileSpec::SetFile (const char *pathname, bool resolve, PathSyntax syntax)
     llvm::SmallString<64> resolved(pathname);
 
     if (resolve)
     {
         FileSpec::Resolve (resolved);
         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)
     {
         m_filename.SetString(resolve_path_ref);
         return;
     }
 

lldb/trunk/unittests/Host/FileSpecTest.cpp

 {
     FileSpec fs_posix("/foo/bar", false, FileSpec::ePathSyntaxPosix);
     EXPECT_STREQ("/foo/bar", fs_posix.GetCString());
     EXPECT_STREQ("/foo", fs_posix.GetDirectory().GetCString());
     EXPECT_STREQ("bar", fs_posix.GetFilename().GetCString());
 
     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);
     EXPECT_STREQ("/", fs_posix_root.GetCString());
     EXPECT_EQ(nullptr, fs_posix_root.GetDirectory().GetCString());
     EXPECT_STREQ("/", fs_posix_root.GetFilename().GetCString());
 
     FileSpec fs_windows_drive("F:", false, FileSpec::ePathSyntaxWindows);
     EXPECT_STREQ("F:", fs_windows_drive.GetCString());
     EXPECT_EQ(nullptr, fs_windows_drive.GetDirectory().GetCString());
     EXPECT_STREQ("F:", fs_windows_drive.GetFilename().GetCString());
 
     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);
     EXPECT_STREQ("/foo/bar/.", fs_posix_trailing_slash.GetCString());
     EXPECT_STREQ("/foo/bar", fs_posix_trailing_slash.GetDirectory().GetCString());
     EXPECT_STREQ(".", fs_posix_trailing_slash.GetFilename().GetCString());
 
     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());
 }
 
 TEST(FileSpecTest, AppendPathComponent)
 {
     FileSpec fs_posix("/foo", false, FileSpec::ePathSyntaxPosix);
     fs_posix.AppendPathComponent("bar");
     EXPECT_STREQ("/foo/bar", fs_posix.GetCString());
     EXPECT_STREQ("/foo", fs_posix.GetDirectory().GetCString());
     EXPECT_STREQ("bar", fs_posix.GetFilename().GetCString());
 
     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);
     fs_posix_root.AppendPathComponent("bar");
     EXPECT_STREQ("/bar", fs_posix_root.GetCString());
     EXPECT_STREQ("/", fs_posix_root.GetDirectory().GetCString());
     EXPECT_STREQ("bar", fs_posix_root.GetFilename().GetCString());
 
     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());
 }
 
 TEST(FileSpecTest, CopyByAppendingPathComponent)
 {
     FileSpec fs = FileSpec("/foo", false, FileSpec::ePathSyntaxPosix).CopyByAppendingPathComponent("bar");
     EXPECT_STREQ("/foo/bar", fs.GetCString());
     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));
+}