[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

This is almost ready. After we're done with this round of cosmetics, I'll take another look a the algorithm and sign off.



================
Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:18-19
+      return true;
+    else if (m_ptr_size == 8 && m_ht_64)
+      return true;
+  }
----------------
`else` after return


================
Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:34-38
+  if (m_ptr_size == 4) {
+    return UpdateFor(m_ht_32);
+  } else if (m_ptr_size == 8) {
+    return UpdateFor(m_ht_64);
+  }
----------------
else after return. no need for `{}`


================
Comment at: lldb/source/Plugins/Language/ObjC/NSDictionary.cpp:753-776
+
+    while (tries < num_children) {
+      key_at_idx = m_keys_ptr + (test_idx * m_ptr_size);
+      val_at_idx = m_values_ptr + (test_idx * m_ptr_size);
+
+      key_at_idx = process_sp->ReadPointerFromMemory(key_at_idx, error);
+      if (error.Fail())
----------------
Can you add a comment explaining what this loop does?


================
Comment at: lldb/source/Plugins/Language/ObjC/NSSet.cpp:653
+    default:
+      assert(false && "pointer size is not 4 nor 8 - get out of here ASAP");
+    }
----------------
lldbassert. Also no need for the second part of the comment.


================
Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py:23
     def nscontainers_data_formatter_commands(self):
+        
         self.expect(
----------------
stray line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78396