labath marked 5 inline comments as done.
labath added inline comments.

================
Comment at: lldb/include/lldb/Utility/FileSpec.h:196
 
   static bool Equal(const FileSpec &a, const FileSpec &b, bool full);
 
----------------
clayborg wrote:
> JDevlieghere wrote:
> > Why do we still need the `Equal` method? Are there cases where `full` is 
> > only decided at runtime? Would it be worth to update the call sites to use 
> > `==` or `::Match` directly? I think having both `Match` and `Equal` with 
> > these semantics is confusing and will likely reintroduce the things you 
> > just cleaned up. 
> I agree with JDev here.
I also wanted to delete it completely, but then I ran into 
FileSpecList::FindFirstFile, which forwards the `full` parameter to this 
function. I believe the calls to *that* function all have "static" values of 
the `full` argument, but the way this argument is used in this function is so 
convoluted, I thought I'd be best to leave that for a separate patch. (I'm 
pretty sure the convolutedness is not intentional, but also a result of the 
misunderstanding of how FileSpec::Equal works, but that means that fixing 
*that* will not be NFC.


================
Comment at: lldb/include/lldb/Utility/FileSpec.h:202
+  /// pattern matches everything.
+  static bool Match(const FileSpec &pattern, const FileSpec &file);
+
----------------
clayborg wrote:
> Maybe rename to "Matches"?
`Match` seems to be the prevalent choice in other match-like apis (`re.match`, 
`pcre_match`, etc.).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70851/new/

https://reviews.llvm.org/D70851



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to