Simplify UUID::IsValid()

[sas]

Summary

This change also prevents looking further than the size of the current
UUID.

[zturner]

You could use llvm's range adapters to make this slightly better.

auto Bytes = makeArrayRef(m_uuid, m_num_uuid_bytes);
return llvm::find(Bytes, 0) != Bytes.end();

Another option would just be return *this != UUID(m_num_uuid_bytes);

[davide]

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.

[clayborg]

A better solution would be to initialize UUID::m_num_uuid_bytes with zero and only set it to a valid value if we set bytes into it. Then UUID::IsValid() becomes easy:

bool UUID::IsValid() const { return m_num_uuid_bytes > 0; }

This would allows us to actually have a UUID value that is valid and all zeroes. A few comments would need to be fixed as it currently assumes length is 16 or 20.

[sas]

In D40537#937196, @zturner wrote:

You could use llvm's range adapters to make this slightly better.

auto Bytes = makeArrayRef(m_uuid, m_num_uuid_bytes);
return llvm::find(Bytes, 0) != Bytes.end();

Another option would just be return *this != UUID(m_num_uuid_bytes);

We want at least one non-zero element, we don't want a zero-element, so the proposed code wouldn't work. I'm not sure if there's an llvm utility that allows doing that directly without having to pass a lambda.

[sas]

In D40537#937862, @clayborg wrote:

A better solution would be to initialize UUID::m_num_uuid_bytes with zero and only set it to a valid value if we set bytes into it. Then UUID::IsValid() becomes easy:

bool UUID::IsValid() const { return m_num_uuid_bytes > 0; }

This would allows us to actually have a UUID value that is valid and all zeroes. A few comments would need to be fixed as it currently assumes length is 16 or 20.

Yes but the current default constructor of the UUID class creates a 16-bytes all-zeroes UUID. I'm not sure I want to be changing the default behavior that the rest of lldb might be depending on currently.

[zturner]

In D40537#937866, @sas wrote:

In D40537#937196, @zturner wrote:

You could use llvm's range adapters to make this slightly better.

auto Bytes = makeArrayRef(m_uuid, m_num_uuid_bytes);
return llvm::find(Bytes, 0) != Bytes.end();

Another option would just be return *this != UUID(m_num_uuid_bytes);

We want at least one non-zero element, we don't want a zero-element, so the proposed code wouldn't work. I'm not sure if there's an llvm utility that allows doing that directly without having to pass a lambda.

Wouldn't the other alternative work, where you just use operator== against a default constructed instance?

[sas]

In D40537#937880, @zturner wrote:

In D40537#937866, @sas wrote:

In D40537#937196, @zturner wrote:

You could use llvm's range adapters to make this slightly better.

auto Bytes = makeArrayRef(m_uuid, m_num_uuid_bytes);
return llvm::find(Bytes, 0) != Bytes.end();

Another option would just be return *this != UUID(m_num_uuid_bytes);

We want at least one non-zero element, we don't want a zero-element, so the proposed code wouldn't work. I'm not sure if there's an llvm utility that allows doing that directly without having to pass a lambda.

Wouldn't the other alternative work, where you just use operator== against a default constructed instance?

The other alternative seems a bit less explicit to me but I don't mind it too much. What's the issue with using std::any_of exactly?

[clayborg]

No one is relying on the 16 bytes of zeroes that I know of. UUID::IsValid() is the test that everyone uses to tell if the UUID is valid or not. I still prefer to just set the length to zero as this does allow us to have a UUID of all zeroes if needed.

source/Utility/UUID.cpp

 #include "lldb/Utility/UUID.h"
 
 // Other libraries and framework includes
 // Project includes
 #include "lldb/Utility/Stream.h"
 #include "llvm/ADT/StringRef.h"
 
 // C Includes
+// C++ Includes
 #include <ctype.h>
 #include <stdio.h>
 #include <string.h>
+#include <algorithm>
+#include <iterator>
 
 namespace lldb_private {
 
 UUID::UUID() { Clear(); }
 
 UUID::UUID(const UUID &rhs) {
   SetBytes(rhs.m_uuid, rhs.m_num_uuid_bytes);
 }
bool UUID::SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes) {
   }
   ::memset(m_uuid, 0, sizeof(m_uuid));
   return false;
 }
 
 size_t UUID::GetByteSize() const { return m_num_uuid_bytes; }
 
 bool UUID::IsValid() const {
-  return m_uuid[0] || m_uuid[1] || m_uuid[2] || m_uuid[3] || m_uuid[4] ||
-         m_uuid[5] || m_uuid[6] || m_uuid[7] || m_uuid[8] || m_uuid[9] ||
-         m_uuid[10] || m_uuid[11] || m_uuid[12] || m_uuid[13] || m_uuid[14] ||
-         m_uuid[15] || m_uuid[16] || m_uuid[17] || m_uuid[18] || m_uuid[19];
+  return std::any_of(std::begin(m_uuid), std::begin(m_uuid) + m_num_uuid_bytes,
+                     [](uint32_t val) { return val != 0; });
 }
 
 static inline int xdigit_to_int(char ch) {
   ch = tolower(ch);
   if (ch >= 'a' && ch <= 'f')
     return 10 + ch - 'a';
   return ch - '0';
 }