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

[Enrico Granata via lldb-commits]

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 @@
+HardcodedFormatters::HardcodedSummaryFinder
+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.


Repository:
  rL LLVM

http://reviews.llvm.org/D13878