[Lldb-commits] [PATCH] D13878: Add data formatters for go strings and slices.

Enrico Granata via lldb-commits lldb-commits at lists.llvm.org
Mon Oct 19 16:40:40 PDT 2015

granata.enrico added inline comments.

Comment at: source/Plugins/Language/Go/GoFormatterFunctions.cpp:104
@@ +103,3 @@
+    size_t m_len;
+    std::vector<lldb::ValueObjectSP> m_children;
This worries me a little bit

Imagine I have one of these slices with a million elements, and the user gets in and the first thing they do is GetChildAtIndex(750000); then GetChildAtIndex(134621); then GetChildAtIndex(999999)

Now I have allocated one million shared pointers, even though only three are in use

How about std::map<size_t, ValueObjectSP>?

Comment at: source/Plugins/Language/Go/GoFormatterFunctions.cpp:145
@@ +144,3 @@
+    StringPrinter::ReadStringAndDumpToStreamOptions options(valobj);
+    options.SetLocation(valobj_addr);
I would also do options.SetLanguage(eLanguageTypeGo) here - in case you ever want to do custom escaping, or the StringPrinter gains any further language-specific abilities

Comment at: source/Plugins/Language/Go/GoLanguage.cpp:79
@@ +78,3 @@
+GoLanguage::GetHardcodedSummaries ()
Any reason why you need to use hardcoded formatters here?

Do strings and slices not have simple type names to match against in Go?

If at all possible, I would prefer to see you add formatters by name instead of hardcoded matches. Hardcoded matches are really meant for cases where the predicate you're trying to express is something a type name or regular expression on type names can't capture.



More information about the lldb-commits mailing list