[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);
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 @@
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