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