yuanzi marked 3 inline comments as done.
yuanzi added inline comments.
================
Comment at: lldb/source/Host/common/XML.cpp:157-159
+ std::string attr_str = GetAttributeValue(name, "");
+ llvm::StringRef attr(attr_str);
+ return llvm::to_integer(attr, value, base);
----------------
labath wrote:
> I don't see why we need these temporary variables. std::string is implicitly
> convertible to a StringRef, and you don't need the value afterwards, so you
> should be able to pass the GetAttributeValue straight into the to_integer
> function. Did that not work for some reason?
I did that mainly to ensure the new code behaves exactly the same as the old.
Since no complex functions are called, they should work interchangeably!
Reverted to previous version.
================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4534
+ std::string main_lm_str = root_element.GetAttributeValue("main-lm");
+ llvm::StringRef main_lm(main_lm_str);
// FIXME: we're silently ignoring invalid data here
----------------
labath wrote:
> Same here. `.empty()` and `to_integer` should work on std::string as well.
> Constructing a StringRef might make sense if we needed to perform some more
> complex operations (ones which std::string does not support) but I don't see
> anything like that here.
Right! Done.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116707/new/
https://reviews.llvm.org/D116707
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
