[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

Thu May 7 07:54:19 PDT 2020

teemperor added a comment.

I don't really know that ValueObject code, but I wish we could just pretend that the dereferenced type of X is a type named Y or something like that and then just map *Ref classes to their non-opaque bridged types when dereferenced. No idea if that's possible though.

Anyway, if we go with the fake empty struct approach then I wish we could just use the (fixed?) list <https://developer.apple.com/library/archive/documentation/CoreFoundation/Conceptual/CFDesignConcepts/Articles/tollFreeBridgedTypes.html#//apple_ref/doc/uid/TP40010677> of these special bridged structs and hard-code them into some ObjC plugin or something like that. If those few selected classes are manually completed as empty structs when we find them in the debug info then I think that's a manageable workaround to get this running.

================
Comment at: lldb/source/Core/ValueObject.cpp:665
   bool child_is_deref_of_parent = false;
+  CompilerType compiler_type = GetCompilerType();
   uint64_t language_flags = 0;
----------------
This change (and the one below) don't seem to be some NFC refactoring? Not sure why this was refactored as `compiler_type` is only ever used once where we previously called `GetCompilerType()`?

================
Comment at: lldb/source/Core/ValueObject.cpp:2853
+            ConstString g___lldb_opaque_ptr_type(
+                compiler_type.GetTypeName().AsCString());
+
----------------
This is the name of the typedef, not the underlying incomplete struct. So this creates a new empty record with the name of the typedef which seems weird. If we fake that some type is actually empty instead of incomplete, then I think the underlying struct makes more sense to emulate. Also this variable name is kinda weird with it's `g____` prefix even though it's not a global or a static.

================
Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py:58

+                
+        self.expect(
----------------
Trailing spaces (that Phabricator somehow doesn't properly highlight :/)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554