[lldb] s/FileSpec::Equal/FileSpec::Match

Summary:
The FileSpec class is often used as a sort of a pattern -- one specifies
a bare file name to search, and we check if in matches the full file
name of an existing module (for example).

These comparisons used FileSpec::Equal, which had some support for it
(via the full=false argument), but it was not a good fit for this job.

For one, it did a symmetric comparison, which makes sense for a function
called "equal", but not for typical searches (when searching for
"/foo/bar.so", we don't want to find a module whose name is just
"bar.so"). This resulted in patterns like:
    if (FileSpec::Equal(pattern, file, pattern.GetDirectory()))
which would request a "full" match only if the pattern really contained
a directory. This worked, but the intended behavior was very unobvious.

On top of that, a lot of the code wanted to handle the case of an
"empty" pattern, and treat it as matching everything. This resulted in
conditions like:
    if (pattern && !FileSpec::Equal(pattern, file, pattern.GetDirectory())
which are nearly impossible to decipher.

This patch introduces a FileSpec::Match function, which does exactly
what most of FileSpec::Equal callers want, an asymmetric match between a
"pattern" FileSpec and a an actual FileSpec. Empty paterns match
everything, filename-only patterns match only the filename component.

I've tried to update all callers of FileSpec::Equal to use a simpler
interface. Those that hardcoded full=true have been changed to use
operator==. Those passing full=pattern.GetDirectory() have been changed
to use FileSpec::Match.

There was also a handful of places which hardcoded full=false. I've
changed these to use FileSpec::Match too. This is a slight change in
semantics, but it does not look like that was ever intended, and it was
more likely a result of a misunderstanding of the "proper" way to use
FileSpec::Equal.

[In an ideal world a "FileSpec" and a "FileSpec pattern" would be two
different types, but given how widespread FileSpec is, it is unlikely
we'll get there in one go. This at least provides a good starting point
by centralizing all matching behavior.]

Reviewers: teemperor, JDevlieghere, jdoerfert

Subscribers: emaste, lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D70851

1 files changed, 2 insertions, 10 deletions

diff --git a/lldb/source/Commands/CommandObjectSource.cpp b/lldb/source/Commands/CommandObjectSource.cpp
index f2591b4f625..807c04f4c65 100644
--- a/lldb/source/Commands/CommandObjectSource.cpp
+++ b/lldb/source/Commands/CommandObjectSource.cpp
@@ -146,12 +146,6 @@ protected:
     Target *target = m_exe_ctx.GetTargetPtr();
 
     uint32_t num_matches = 0;
-    bool has_path = false;
-    if (file_spec) {
-      assert(file_spec.GetFilename().AsCString());
-      has_path = (file_spec.GetDirectory().AsCString() != nullptr);
-    }
-
     // Dump all the line entries for the file in the list.
     ConstString last_module_file_name;
     uint32_t num_scs = sc_list.GetSize();
@@ -168,8 +162,7 @@ protected:
         if (module_list.GetSize() &&
             module_list.GetIndexForModule(module) == LLDB_INVALID_INDEX32)
           continue;
-        if (file_spec && !lldb_private::FileSpec::Equal(
-                             file_spec, line_entry.file, has_path))
+        if (!FileSpec::Match(file_spec, line_entry.file))
           continue;
         if (start_line > 0 && line_entry.line < start_line)
           continue;
@@ -250,8 +243,7 @@ protected:
             num_matches++;
             if (num_lines > 0 && num_matches > num_lines)
               break;
-            assert(lldb_private::FileSpec::Equal(cu_file_spec, line_entry.file,
-                                                 has_path));
+            assert(cu_file_spec == line_entry.file);
             if (!cu_header_printed) {
               if (num_matches > 0)
                 strm << "\n\n";