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

[Frederic Riss via Phabricator via lldb-commits]

friss added inline comments.


================
Comment at: lldb/source/Core/ValueObject.cpp:52
 
+#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
+
----------------
You don't want to introduce a dependency between Core and a plugin. What you need here might need to be introduced as a new abstract functionality on the TypeSystem base class.


================
Comment at: lldb/source/Core/ValueObject.cpp:2846
+      if (llvm::isa<TypeSystemClang>(compiler_type.GetTypeSystem())) {
+        if (HasSyntheticValue()) {
+          TargetSP target_sp = GetTargetSP();
----------------
I am understanding correctly that this condition will only trigger when there's a synthetic child provider for the type? In other words, if we have an opaque pointer that we don't know how to display, it will still cause an error, right?


================
Comment at: lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCNSContainer.py:59-60
+                
+        self.expect(
+            'frame variable -d run-target *cfDictionaryRef',
+            patterns=[
----------------
I must be missing something obvious, but it seems like that patch doesn't register a formatter for CFDictionaryRef. Was it already there but non-functional?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79554