JDevlieghere added a comment.
The relative/absolute caching seems like a nice improvement. Splitting it into
a separate patch would make it easier to review.
I'm slightly worried about the change to make the new "fuzzy" matching the
default. While it makes sense for the breakpoints, I wouldn't generally expect
`./a/b/c/main.cpp` to match `/build/a/b/c/main.cpp`, at least not unless my
current working directory is `/build`. While doing the reproducers, the CWD was
probably the hardest thing to get right consistently. I'm worried that this new
matching is going to lead to really subtle bugs that are going to be tricky to
figure out.
To make this more obvious and explicit: would it make sense to have another
matches in FileSpec and have FindFileIndex take a `std::function` or
`llvm::FunctionRef` with the desired matcher. Something like:
static bool FuzzyMatch(const FileSpec &pattern, const FileSpec &file);
size_t FileSpecList::FindFileIndex(size_t start_idx, const FileSpec
&file_spec,
bool full, std::function<bool(const
FileSpec &, const FileSpec &)> matcher = FileSpec::Match) const {
...
}
================
Comment at: lldb/include/lldb/Utility/FileSpec.h:408-409
mutable bool m_is_resolved = false; ///< True if this path has been resolved.
+ mutable bool m_is_absolute = false; ///< True if this path is absolute.
+ mutable bool m_is_absolute_valid = false; ///< True if m_is_absolute is
valid.
Style m_style; ///< The syntax that this path uses (e.g. Windows / Posix)
----------------
Should this be an `llvm::Optional<bool>` instead or maybe even better an enum
value with values `absolute, relative, unknown` or something?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130045/new/
https://reviews.llvm.org/D130045
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
