bulbazord added a comment.
Ok, this is a pretty big patch so we probably don't want to leave it in review
for a long time or it just gets harder to land correctly.
I've looked over most of this patch and it looks fine to me. A lot of is rather
mechanical: Changing things like `GetIntegerValue` to `GetUnsignedIntegerValue`
and using `GetSignedIntegerValue` where it makes sense. There are lots of
little cleanups as well, like correcting the type of some variables (e.g.
`addr_t` -> `offset_t`) and these are fine too. I don't have much of a problem
with any of that.
2 small things though and I think this should probably be good after that.
================
Comment at: lldb/include/lldb/API/SBStructuredData.h:75-78
+ // LLDB_DEPRECATED("Specify if the value is signed or unsigned",
+ // "uint64_t GetUnsignedIntegerValue(uint64_t fail_value =
+ // 0)")
+ [[deprecated("Specify if the value is signed or unsigned")]] uint64_t
----------------
Did you mean to remove the deprecated attribute and use `LLDB_DEPRECATED`
directly?
================
Comment at: lldb/include/lldb/Utility/StructuredData.h:105-106
+ UnsignedInteger *GetAsUnsignedInteger() {
+ return ((m_type == lldb::eStructuredDataTypeInteger ||
+ m_type == lldb::eStructuredDataTypeUnsignedInteger)
+ ? static_cast<UnsignedInteger *>(this)
----------------
Could you add a small note here specifying why `eStructuredDataTypeInteger`
corresponds to UnsignedInteger and not SignedInteger?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150485/new/
https://reviews.llvm.org/D150485
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
