JDevlieghere added a comment.
I figured I'd leave some nits on a Friday afternoon.
================
Comment at: lldb/bindings/interface/SBValue.i:117-132
+" // Return the value as an address. On failure, LLDB_INVALID_ADDRESS
+ // will be returned. On architectures like AArch64, where the top
+ // (unaddressable) bits can be used for authentication, memory tagging,
+ // or top byte ignore, this method will return the value with those
+ // top bits cleared.
+ //
+ // GetValueAsUnsigned returns the actual value, with the
----------------
Nit: no `//`
================
Comment at: lldb/source/API/SBValue.cpp:932-934
+ if (!success) {
+ return fail_value;
+ }
----------------
Nit: no braces around single life if for consistency with lines 935 and 393.
================
Comment at: lldb/source/API/SBValue.cpp:935-940
+ ProcessSP process_sp = m_opaque_sp->GetProcessSP();
+ if (!process_sp)
+ return ret_val;
+ ABISP abi_sp = process_sp->GetABI();
+ if (abi_sp)
+ return abi_sp->FixCodeAddress(ret_val);
----------------
Nit^3: why not do the same as in `GetStrippedPointerValue` and
`CreateValueObjectFromAddress`:
```
if (ProcessSP process_sp = m_opaque_sp->GetProcessSP())
if (ABISP abi_sp = process_sp->GetABI())
return abi_sp->FixCodeAddress(ret_val);
```
================
Comment at: lldb/source/API/SBValue.cpp:938-940
+ ABISP abi_sp = process_sp->GetABI();
+ if (abi_sp)
+ return abi_sp->FixCodeAddress(ret_val);
----------------
Nit^2: in other places where I upstreamed `FixCodeAddress` calls I went with
the following LLVM pattern:
```
if (ABISP abi_sp = process_sp->GetABI())
return abi_sp->FixCodeAddress(ret_val);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142792/new/
https://reviews.llvm.org/D142792
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
