aprantl added inline comments.
================
Comment at: lldb/source/Target/PathMappingList.cpp:33
// nomalized path pairs to ensure things match up.
ConstString NormalizePath(ConstString path) {
// If we use "path" to construct a FileSpec, it will normalize the path for
----------------
Why does this take a ConstString argument if it immediately calls GetStringRef
on it?
Could you change this to take a StringRef here or in a follow-up NFC commit?
================
Comment at: lldb/source/Target/PathMappingList.cpp:33-37
ConstString NormalizePath(ConstString path) {
// If we use "path" to construct a FileSpec, it will normalize the path for
// us. We then grab the string and turn it back into a ConstString.
return ConstString(FileSpec(path.GetStringRef()).GetPath());
}
----------------
xujuntwt95329 wrote:
> JDevlieghere wrote:
> > Can this function take a `StringRef` and return a `std::string` instead?
> > The amount of roundtrips between `StringRef`s, `ConstString`s and
> > `std::string`s is getting a bit out of hand.
> I agree with you.
> However, if we change the signature of this function, then we need to do all
> these conversion from the caller side, seems things are not going better.
>
> For example
>
> ```
> void PathMappingList::Append(ConstString path,
> ConstString replacement, bool notify) {
> ++m_mod_id;
> m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
> if (notify && m_callback)
> m_callback(*this, m_callback_baton);
> }
> ```
>
> We need to convert `path` to StringRef, or we can change the type of
> parameter `path`, but this will require more modification from the caller of
> `Append`
IMO, the best solution would be change this too:
```
void PathMappingList::Append(StringRef path,
StringRef replacement, bool notify) {
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112439/new/
https://reviews.llvm.org/D112439
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
