[Lldb-commits] [PATCH] D55584: [LLDB] Simplify Boolean expressions

[Shafik Yaghmour via Phabricator via lldb-commits]

shafik accepted this revision as: shafik.
shafik added a comment.

LGTM after comment are addressed.



================
Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5894
+    if (!success) {
+      offset = MachHeaderSizeFromMagic(m_header.magic);
+      for (uint32_t i = 0; !success && i < m_header.ncmds; ++i) {
----------------
This looks like a big chunk of unrelated changes.


================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1725
     // not support qHostInfo or qWatchpointSupportInfo packets.
-    if (atype == llvm::Triple::mips || atype == llvm::Triple::mipsel ||
-        atype == llvm::Triple::mips64 || atype == llvm::Triple::mips64el ||
-        atype == llvm::Triple::ppc64le)
-      after = false;
-    else
-      after = true;
+    after =
+        !(atype == llvm::Triple::mips || atype == llvm::Triple::mipsel ||
----------------
Not sure if I like this one, the previous form feels more readable.


================
Comment at: source/Plugins/Process/mach-core/ProcessMachCore.cpp:305
   std::string corefile_identifier = core_objfile->GetIdentifierString();
-  if (found_main_binary_definitively == false 
-      && corefile_identifier.find("Darwin Kernel") != std::string::npos) {
-      UUID uuid;
-      addr_t addr = LLDB_INVALID_ADDRESS;
-      if (corefile_identifier.find("UUID=") != std::string::npos) {
-          size_t p = corefile_identifier.find("UUID=") + strlen("UUID=");
-          std::string uuid_str = corefile_identifier.substr(p, 36);
-          uuid.SetFromStringRef(uuid_str);
-      }
-      if (corefile_identifier.find("stext=") != std::string::npos) {
-          size_t p = corefile_identifier.find("stext=") + strlen("stext=");
-          if (corefile_identifier[p] == '0' && corefile_identifier[p + 1] == 'x') {
-              errno = 0;
-              addr = ::strtoul(corefile_identifier.c_str() + p, NULL, 16);
-              if (errno != 0 || addr == 0)
-                  addr = LLDB_INVALID_ADDRESS;
-          }
-      }
-      if (uuid.IsValid() && addr != LLDB_INVALID_ADDRESS) {
-          m_mach_kernel_addr = addr;
-          found_main_binary_definitively = true;
-          if (log)
-            log->Printf("ProcessMachCore::DoLoadCore: Using the kernel address 0x%" PRIx64
-                        " from LC_IDENT/LC_NOTE 'kern ver str' string: '%s'", addr, corefile_identifier.c_str());
+  if (!found_main_binary_definitively &&
+      corefile_identifier.find("Darwin Kernel") != std::string::npos) {
----------------
Another big chunk of unrelated changes.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp:1117
       if ((distance(range.first, range.second) == 1) &&
-          range.first->end_sequence == true) {
+          static_cast<bool>(range.first->end_sequence)) {
         *range.first = state;
----------------
The `static_cast` feels less readable here.


================
Comment at: source/Symbol/LineTable.cpp:217
                 if (prev_pos->file_addr == search_entry.file_addr &&
-                    prev_pos->is_terminal_entry == false)
+                    !static_cast<bool>(prev_pos->is_terminal_entry))
                   --pos;
----------------
static_cast


================
Comment at: source/Symbol/LineTable.cpp:236
         // terminating entry for a previous line...
-        if (pos != end_pos && pos->is_terminal_entry == false) {
+        if (pos != end_pos && !static_cast<bool>(pos->is_terminal_entry)) {
           uint32_t match_idx = std::distance(begin_pos, pos);
----------------
static_cast


================
Comment at: source/Symbol/Type.cpp:749
 bool TypeAndOrName::IsEmpty() const {
-  if ((bool)m_type_name || (bool)m_type_pair)
-    return false;
-  else
-    return true;
+  return !((bool)m_type_name || (bool)m_type_pair);
 }
----------------
Oh, even worse C-style casts ... can we remove these. I am assuming they are triggering a member conversion function.


================
Comment at: source/Utility/RegisterValue.cpp:478
   if (this == &rhs)
-    return rhs.m_type == eTypeInvalid ? false : true;
+    return rhs.m_type != eTypeInvalid;
 
----------------
Nice one.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55584/new/

https://reviews.llvm.org/D55584