[Lldb-commits] [PATCH] D40537: Simplify UUID::IsValid()

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Tue Nov 28 10:18:25 PST 2017


> On Nov 27, 2017, at 10:11 PM, Zachary Turner <zturner at google.com> wrote:
> 
> As an aside, I don't really like this class.  For example, You can currently assign a UUID[16] to a UUID[20].  That doesn't make a lot of sense to me.

What about an invalid UUID[0] being assigned with a valid UUID[16] or UUID[20]? Why doesn't this make sense? I don't follow.

> 
> As a future cleanup, I think this class should probably be a template such as UUID<N>, and then internally it can store a std::array<uint8_t, N>.  And we can static_assert that N is of a known size if we desire.

UUID values are objects contained as members inside of other objects. They all default to start with no preconceived notion of what the UUID should be. IMHO the UUID class is just fine and needs to be able to represent any UUID, from empty uninitialized ones, and be able to be assigned and changed at will.

Greg

> 
> On Mon, Nov 27, 2017 at 9:38 PM Davide Italiano via Phabricator <reviews at reviews.llvm.org <mailto:reviews at reviews.llvm.org>> wrote:
> davide added a comment.
> 
> Yes, what Zachary said. Thanks for the cleanup. There's a decent amount of code in lldb that can be written in a very compact way but it's manually unrolled. I think in this case the code produced should be equivalent (and if not, I'd be curious to see the codegen).
> More generally, whenever a function is not in a hot loop, we might consider preferring readability over performances anyway.
> 
> 
> https://reviews.llvm.org/D40537 <https://reviews.llvm.org/D40537>
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20171128/48dec204/attachment.html>


More information about the lldb-commits mailing list