lemo added a comment.
> The slight complication here is that
> some clients (MachO) actually use the all-zero notation to mean "no UUID
> has been set". To keep this use case working, I have introduced an
> additional argument to the UUID constructor, which specifies whether an
> all-zero vector should be considered a valid UUID. For the usages where
> the UUID data comes from a MachO file, I set this argument to false.
What is the distinction between "no UUID has been set" and "invalid UUID"? I
find this subtlety confusing, can we simplify it to just valid/not-valid UUIDs?
================
Comment at: include/lldb/Utility/UUID.h:31
// Most UUIDs are 16 bytes, but some Linux build-ids (SHA1) are 20.
typedef uint8_t ValueType[20];
----------------
switch to llvm::SmallVector<uint8_t, 20> as suggested by Greg?
================
Comment at: include/lldb/Utility/UUID.h:42
- const UUID &operator=(const UUID &rhs);
+ UUID &operator=(const UUID &rhs) = default;
----------------
If we define this we should define at least the copy constructor as well (rule
of 3/5). In this case the cleanest solution may be to allow the implicit
definitions (which would also allow the implicit move operations - defining
assignment operator as defaulted would inhibit the implicit move SMFs)
================
Comment at: include/lldb/Utility/UUID.h:52
- bool IsValid() const;
+ explicit operator bool() const { return IsValid(); }
+ bool IsValid() const { return m_num_uuid_bytes > 0; }
----------------
is this really needed? I'd prefer the truly explicit (pun intended) IsValid()
================
Comment at: include/lldb/Utility/UUID.h:95
bool operator==(const UUID &lhs, const UUID &rhs);
bool operator!=(const UUID &lhs, const UUID &rhs);
----------------
these can be simplified if we use llvm::SmallVector (or std::vector)
https://reviews.llvm.org/D48479
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
