JDevlieghere added a comment.
In D158041#4593205 <https://reviews.llvm.org/D158041#4593205>, @jasonmolenda
wrote:
> Update patch to change the `AddressableBits::IsValid` to
> `AddressableBits::HasValue` which is a clearer description. The `IsValid`
> methods are used across the lldb_private classes everywhere. I tried
> changing this to `operator bool` but I thought that was less clear in use. I
> compromised by changing it to `HasValue` - this is a POD class so "IsValid"
> seemed a little questionable, but I didn't like operator bool. Let's try
> this out.
Yeah that's fine. I don't feel strongly about `operator bool()`. My main
concern is to not create another class that can be in an "invalid state" and
requires you to check whether or not it's valid before doing an operation. As
you point out, that's not really the case here anyway, and using different
terminology helps convey that.
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:2314-2316
+ if (addressable_bits.HasValue()) {
+ addressable_bits.SetProcessMasks(*this);
+ }
----------------
I would inline the check for `HasValue` in `SetProcessMasks`. You already do
some kind of sanity checking there, might as well make that sound and do it
unconditionally.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158041/new/
https://reviews.llvm.org/D158041
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
