<div dir="ltr">Thanks for the clarification!</div><br><div class="gmail_quote"><div dir="ltr">On Tue, Oct 6, 2015 at 10:51 PM Enrico Granata <<a href="mailto:egranata@apple.com">egranata@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto"><div>I am not done here.</div><div>Also, even once I am done, all that you will be able to see here is no regressions in existing functionality.</div><div><br></div><div>I am plumbing through a set of facilities that I need in order to make certain parts of data formatting logic Language-plugin based.</div><div><br></div><div>That is required to make it easier to support new source languages in a clean fashion.</div><div><br></div><div>Since the C++ and ObjC support is already there, there will be no (unintended) change in functionality. What you want to see here testing-wise is the lack of any regression for existing languages.</div><div><br></div><div>The benefits are to be reaped if you were to add support for a new source language, which then would require its own testing, ..., but I plan to do no such thing at the moment.</div><div><br></div><div><b>tl;dr</b> existing tests cover this already; this is refactoring work</div><div><br></div><div>Sent from my iPhone</div></div><div dir="auto"><div><br>On Oct 6, 2015, at 9:20 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br><br></div><blockquote type="cite"><div><div dir="ltr">Actually upon further inspection it looks like the test that was updated was not really anything new, but an update of an existing test to pass a new argument through.<div><br></div><div>Can you add some tests that test this specific functionality?</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Oct 6, 2015 at 9:09 PM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">It looks like there's only 1 test added for all of this functionality from this and the last few commits, and that the test is specific to Objective C. The functionality itself seems language agnostic though. Is there any way to write a test that does not rely on a particular language? That would improve the test coverage of this functionality.</div><br><div class="gmail_quote"><div dir="ltr">On Tue, Oct 6, 2015 at 7:38 PM Enrico Granata via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: enrico<br>
Date: Tue Oct 6 21:36:35 2015<br>
New Revision: 249507<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=249507&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=249507&view=rev</a><br>
Log:<br>
Route the preferred-display-language mechanism to the ValueObjectPrinter and actually fill in a few gaps for dynamic and synthetic values to be able to adopt this in useful ways<br>
<br>
<br>
Modified:<br>
lldb/trunk/include/lldb/Core/ValueObject.h<br>
lldb/trunk/include/lldb/Core/ValueObjectDynamicValue.h<br>
lldb/trunk/include/lldb/Core/ValueObjectSyntheticFilter.h<br>
lldb/trunk/include/lldb/DataFormatters/ValueObjectPrinter.h<br>
lldb/trunk/source/Commands/CommandObjectExpression.cpp<br>
lldb/trunk/source/Commands/CommandObjectFrame.cpp<br>
lldb/trunk/source/Core/ValueObject.cpp<br>
lldb/trunk/source/Core/ValueObjectConstResult.cpp<br>
lldb/trunk/source/Core/ValueObjectDynamicValue.cpp<br>
lldb/trunk/source/Core/ValueObjectSyntheticFilter.cpp<br>
lldb/trunk/source/DataFormatters/ValueObjectPrinter.cpp<br>
lldb/trunk/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py<br>
<br>
Modified: lldb/trunk/include/lldb/Core/ValueObject.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ValueObject.h?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ValueObject.h?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/include/lldb/Core/ValueObject.h (original)<br>
+++ lldb/trunk/include/lldb/Core/ValueObject.h Tue Oct 6 21:36:35 2015<br>
@@ -1242,6 +1242,9 @@ protected:<br>
bool<br>
IsChecksumEmpty ();<br>
<br>
+ void<br>
+ SetPreferredDisplayLanguageIfNeeded (lldb::LanguageType);<br>
+<br>
private:<br>
//------------------------------------------------------------------<br>
// For ValueObject only<br>
<br>
Modified: lldb/trunk/include/lldb/Core/ValueObjectDynamicValue.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ValueObjectDynamicValue.h?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ValueObjectDynamicValue.h?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/include/lldb/Core/ValueObjectDynamicValue.h (original)<br>
+++ lldb/trunk/include/lldb/Core/ValueObjectDynamicValue.h Tue Oct 6 21:36:35 2015<br>
@@ -105,6 +105,12 @@ public:<br>
virtual TypeImpl<br>
GetTypeImpl ();<br>
<br>
+ virtual lldb::LanguageType<br>
+ GetPreferredDisplayLanguage ();<br>
+<br>
+ void<br>
+ SetPreferredDisplayLanguage (lldb::LanguageType);<br>
+<br>
virtual bool<br>
GetDeclaration (Declaration &decl);<br>
<br>
<br>
Modified: lldb/trunk/include/lldb/Core/ValueObjectSyntheticFilter.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ValueObjectSyntheticFilter.h?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ValueObjectSyntheticFilter.h?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/include/lldb/Core/ValueObjectSyntheticFilter.h (original)<br>
+++ lldb/trunk/include/lldb/Core/ValueObjectSyntheticFilter.h Tue Oct 6 21:36:35 2015<br>
@@ -152,6 +152,12 @@ public:<br>
virtual void<br>
SetFormat (lldb::Format format);<br>
<br>
+ virtual lldb::LanguageType<br>
+ GetPreferredDisplayLanguage ();<br>
+<br>
+ void<br>
+ SetPreferredDisplayLanguage (lldb::LanguageType);<br>
+<br>
virtual bool<br>
GetDeclaration (Declaration &decl);<br>
<br>
<br>
Modified: lldb/trunk/include/lldb/DataFormatters/ValueObjectPrinter.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/DataFormatters/ValueObjectPrinter.h?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/DataFormatters/ValueObjectPrinter.h?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/include/lldb/DataFormatters/ValueObjectPrinter.h (original)<br>
+++ lldb/trunk/include/lldb/DataFormatters/ValueObjectPrinter.h Tue Oct 6 21:36:35 2015<br>
@@ -61,6 +61,7 @@ struct DumpValueObjectOptions<br>
lldb::Format m_format = lldb::eFormatDefault;<br>
lldb::TypeSummaryImplSP m_summary_sp;<br>
std::string m_root_valobj_name;<br>
+ lldb::LanguageType m_varformat_language = lldb::eLanguageTypeUnknown;<br>
PointerDepth m_max_ptr_depth;<br>
bool m_use_synthetic : 1;<br>
bool m_scope_already_checked : 1;<br>
@@ -252,6 +253,13 @@ struct DumpValueObjectOptions<br>
return *this;<br>
}<br>
<br>
+ DumpValueObjectOptions&<br>
+ SetVariableFormatDisplayLanguage (lldb::LanguageType lang = lldb::eLanguageTypeUnknown)<br>
+ {<br>
+ m_varformat_language = lang;<br>
+ return *this;<br>
+ }<br>
+<br>
DumpValueObjectOptions&<br>
SetRunValidator (bool run = true)<br>
{<br>
<br>
Modified: lldb/trunk/source/Commands/CommandObjectExpression.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectExpression.cpp?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectExpression.cpp?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Commands/CommandObjectExpression.cpp (original)<br>
+++ lldb/trunk/source/Commands/CommandObjectExpression.cpp Tue Oct 6 21:36:35 2015<br>
@@ -335,6 +335,7 @@ CommandObjectExpression::EvaluateExpress<br>
result_valobj_sp->SetFormat (format);<br>
<br>
DumpValueObjectOptions options(m_varobj_options.GetAsDumpOptions(m_command_options.m_verbosity,format));<br>
+ options.SetVariableFormatDisplayLanguage(result_valobj_sp->GetPreferredDisplayLanguage());<br>
<br>
result_valobj_sp->Dump(*output_stream,options);<br>
<br>
<br>
Modified: lldb/trunk/source/Commands/CommandObjectFrame.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectFrame.cpp?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectFrame.cpp?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Commands/CommandObjectFrame.cpp (original)<br>
+++ lldb/trunk/source/Commands/CommandObjectFrame.cpp Tue Oct 6 21:36:35 2015<br>
@@ -500,6 +500,7 @@ protected:<br>
}<br>
<br>
options.SetFormat(format);<br>
+ options.SetVariableFormatDisplayLanguage(valobj_sp->GetPreferredDisplayLanguage());<br>
<br>
Stream &output_stream = result.GetOutputStream();<br>
options.SetRootValueObjectName(valobj_sp->GetParent() ? name_cstr : NULL);<br>
@@ -586,6 +587,7 @@ protected:<br>
}<br>
<br>
options.SetFormat(format);<br>
+ options.SetVariableFormatDisplayLanguage(valobj_sp->GetPreferredDisplayLanguage());<br>
options.SetRootValueObjectName(name_cstr);<br>
valobj_sp->Dump(result.GetOutputStream(),options);<br>
}<br>
<br>
Modified: lldb/trunk/source/Core/ValueObject.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObject.cpp?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObject.cpp?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Core/ValueObject.cpp (original)<br>
+++ lldb/trunk/source/Core/ValueObject.cpp Tue Oct 6 21:36:35 2015<br>
@@ -4259,6 +4259,13 @@ ValueObject::SetPreferredDisplayLanguage<br>
m_preferred_display_language = lt;<br>
}<br>
<br>
+void<br>
+ValueObject::SetPreferredDisplayLanguageIfNeeded (lldb::LanguageType lt)<br>
+{<br>
+ if (m_preferred_display_language == lldb::eLanguageTypeUnknown)<br>
+ SetPreferredDisplayLanguage(lt);<br>
+}<br>
+<br>
bool<br>
ValueObject::CanProvideValue ()<br>
{<br>
<br>
Modified: lldb/trunk/source/Core/ValueObjectConstResult.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObjectConstResult.cpp?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObjectConstResult.cpp?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Core/ValueObjectConstResult.cpp (original)<br>
+++ lldb/trunk/source/Core/ValueObjectConstResult.cpp Tue Oct 6 21:36:35 2015<br>
@@ -374,5 +374,7 @@ ValueObjectConstResult::Cast (const Comp<br>
lldb::LanguageType<br>
ValueObjectConstResult::GetPreferredDisplayLanguage ()<br>
{<br>
- return lldb::eLanguageTypeUnknown;<br>
+ if (m_preferred_display_language != lldb::eLanguageTypeUnknown)<br>
+ return m_preferred_display_language;<br>
+ return GetCompilerTypeImpl().GetMinimumLanguage();<br>
}<br>
<br>
Modified: lldb/trunk/source/Core/ValueObjectDynamicValue.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObjectDynamicValue.cpp?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObjectDynamicValue.cpp?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Core/ValueObjectDynamicValue.cpp (original)<br>
+++ lldb/trunk/source/Core/ValueObjectDynamicValue.cpp Tue Oct 6 21:36:35 2015<br>
@@ -391,6 +391,27 @@ ValueObjectDynamicValue::SetData (DataEx<br>
return ret_val;<br>
}<br>
<br>
+void<br>
+ValueObjectDynamicValue::SetPreferredDisplayLanguage (lldb::LanguageType lang)<br>
+{<br>
+ this->ValueObject::SetPreferredDisplayLanguage(lang);<br>
+ if (m_parent)<br>
+ m_parent->SetPreferredDisplayLanguage(lang);<br>
+}<br>
+<br>
+lldb::LanguageType<br>
+ValueObjectDynamicValue::GetPreferredDisplayLanguage ()<br>
+{<br>
+ if (m_preferred_display_language == lldb::eLanguageTypeUnknown)<br>
+ {<br>
+ if (m_parent)<br>
+ return m_parent->GetPreferredDisplayLanguage();<br>
+ return lldb::eLanguageTypeUnknown;<br>
+ }<br>
+ else<br>
+ return m_preferred_display_language;<br>
+}<br>
+<br>
bool<br>
ValueObjectDynamicValue::GetDeclaration (Declaration &decl)<br>
{<br>
<br>
Modified: lldb/trunk/source/Core/ValueObjectSyntheticFilter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObjectSyntheticFilter.cpp?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/ValueObjectSyntheticFilter.cpp?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/Core/ValueObjectSyntheticFilter.cpp (original)<br>
+++ lldb/trunk/source/Core/ValueObjectSyntheticFilter.cpp Tue Oct 6 21:36:35 2015<br>
@@ -223,6 +223,7 @@ ValueObjectSynthetic::GetChildAtIndex (s<br>
if (!synth_guy)<br>
return synth_guy;<br>
m_children_byindex.SetValueForKey(idx, synth_guy.get());<br>
+ synth_guy->SetPreferredDisplayLanguageIfNeeded(GetPreferredDisplayLanguage());<br>
return synth_guy;<br>
}<br>
else<br>
@@ -315,6 +316,27 @@ ValueObjectSynthetic::SetFormat (lldb::F<br>
this->ClearUserVisibleData(eClearUserVisibleDataItemsAll);<br>
}<br>
<br>
+void<br>
+ValueObjectSynthetic::SetPreferredDisplayLanguage (lldb::LanguageType lang)<br>
+{<br>
+ this->ValueObject::SetPreferredDisplayLanguage(lang);<br>
+ if (m_parent)<br>
+ m_parent->SetPreferredDisplayLanguage(lang);<br>
+}<br>
+<br>
+lldb::LanguageType<br>
+ValueObjectSynthetic::GetPreferredDisplayLanguage ()<br>
+{<br>
+ if (m_preferred_display_language == lldb::eLanguageTypeUnknown)<br>
+ {<br>
+ if (m_parent)<br>
+ return m_parent->GetPreferredDisplayLanguage();<br>
+ return lldb::eLanguageTypeUnknown;<br>
+ }<br>
+ else<br>
+ return m_preferred_display_language;<br>
+}<br>
+<br>
bool<br>
ValueObjectSynthetic::GetDeclaration (Declaration &decl)<br>
{<br>
<br>
Modified: lldb/trunk/source/DataFormatters/ValueObjectPrinter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/ValueObjectPrinter.cpp?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/DataFormatters/ValueObjectPrinter.cpp?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/source/DataFormatters/ValueObjectPrinter.cpp (original)<br>
+++ lldb/trunk/source/DataFormatters/ValueObjectPrinter.cpp Tue Oct 6 21:36:35 2015<br>
@@ -26,6 +26,7 @@ DumpValueObjectOptions()<br>
{<br>
m_use_dynamic = valobj.GetDynamicValueType();<br>
m_use_synthetic = valobj.IsSynthetic();<br>
+ m_varformat_language = valobj.GetPreferredDisplayLanguage();<br>
}<br>
<br>
ValueObjectPrinter::ValueObjectPrinter (ValueObject* valobj,<br>
@@ -354,10 +355,10 @@ ValueObjectPrinter::GetValueSummaryError<br>
{<br>
TypeSummaryImpl* entry = GetSummaryFormatter();<br>
if (entry)<br>
- m_valobj->GetSummaryAsCString(entry, summary);<br>
+ m_valobj->GetSummaryAsCString(entry, summary, options.m_varformat_language);<br>
else<br>
{<br>
- const char* sum_cstr = m_valobj->GetSummaryAsCString();<br>
+ const char* sum_cstr = m_valobj->GetSummaryAsCString(options.m_varformat_language);<br>
if (sum_cstr)<br>
summary.assign(sum_cstr);<br>
}<br>
<br>
Modified: lldb/trunk/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py?rev=249507&r1=249506&r2=249507&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py?rev=249507&r1=249506&r2=249507&view=diff</a><br>
==============================================================================<br>
--- lldb/trunk/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py (original)<br>
+++ lldb/trunk/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py Tue Oct 6 21:36:35 2015<br>
@@ -387,7 +387,6 @@ class ObjCDataFormatterTestCase(TestBase<br>
self.addTearDownHook(cleanup)<br>
<br>
# check formatters for common Objective-C types<br>
- self.runCmd("log timers enable")<br>
expect_strings = ['(CFGregorianUnits) cf_greg_units = 1 years, 3 months, 5 days, 12 hours, 5 minutes 7 seconds',<br>
'(CFRange) cf_range = location=4 length=4',<br>
'(NSPoint) ns_point = (x = 4, y = 4)',<br>
@@ -414,7 +413,6 @@ class ObjCDataFormatterTestCase(TestBase<br>
<br>
self.expect("frame variable",<br>
substrs = expect_strings)<br>
- self.runCmd('log timers dump')<br>
<br>
<br>
def kvo_data_formatter_commands(self):<br>
<br>
<br>
_______________________________________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br>
</blockquote></div></blockquote></div>
</div></blockquote></div></blockquote></div>