This is an archive of the discontinued LLVM Phabricator instance.

Added long 'print-values' option for var-update MI command.
ClosedPublic

Authored by ki.stfu on Mar 2 2015, 8:56 AM.

Details

Summary

The -var-update MI command should take the same print-values options as var-list children, however currently only the integer versions are supported.
Added --no-values, --all-values, and --simple-values long options.

See:
https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Variable-Objects.html#GDB_002fMI-Variable-Objects

Patch from ewan@codeplay.com

Diff Detail

Repository
rL LLVM

Event Timeline

EwanCrawford retitled this revision from to Added long 'print-values' option for var-update MI command. .
EwanCrawford updated this object.
EwanCrawford edited the test plan for this revision. (Show Details)
EwanCrawford set the repository for this revision to rL LLVM.
EwanCrawford added a subscriber: Unknown Object (MLST).
ki.stfu requested changes to this revision.Mar 2 2015, 9:46 AM
ki.stfu added a reviewer: ki.stfu.
ki.stfu added a subscriber: ki.stfu.

See my comments.

lldb/tools/lldb-mi/MICmdCmdVar.cpp
339–341 ↗(On Diff #21010)

I think it should be pArgNoValues/pArgAllValues/pArgSimpleValues (aka --no-value*s*/--all-value*s*/--simple-value*s*)

343–357 ↗(On Diff #21010)

Use the following:

CMICmnLLDBDebugSessionInfo::VariableInfoFormat_e eVarInfoFormat;
if (pArgPrintValues->GetFound())
{
    const MIuint nPrintValues = pArgPrintValues->GetValue();
    if (nPrintValues >= CMICmnLLDBDebugSessionInfo::kNumVariableInfoFormats)
    {
        SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_PRINT_VALUES), m_cmdData.strMiCmd.c_str()));
        return MIstatus::failure;
    }
    eVarInfoFormat = static_cast<CMICmnLLDBDebugSessionInfo::VariableInfoFormat_e>(nPrintValues);
}
else if (pArgNoValues->GetFound())
    eVarInfoFormat = CMICmnLLDBDebugSessionInfo::eVariableInfoFormat_NoValues;
else if (pArgAllValues->GetFound())
    eVarInfoFormat = CMICmnLLDBDebugSessionInfo::eVariableInfoFormat_AllValues;
else if (pArgSimpleValues->GetFound())
    eVarInfoFormat = CMICmnLLDBDebugSessionInfo::eVariableInfoFormat_SimpleValues;
else
    // If no print-values, default is "no-values"
    eVarInfoFormat = CMICmnLLDBDebugSessionInfo::eVariableInfoFormat_NoValues;
m_eVarInfoFormat = eVarInfoFormat;
532 ↗(On Diff #21010)

why you are ignoring --simple-value?
In addition you should do the same thing in CMICmdCmdVarUpdate::MIFormResponse.

This revision now requires changes to proceed.Mar 2 2015, 9:46 AM

Thanks for the comments ki.stfu.

My understanding of MIFormResponse() is that it is only called from CMICmdCmdVarUpdate::Execute() when there are composite types, in which case we wouldn't print the values for --simple-values. So we only need to check for --all-values and can ignore --simple-values.
Should MIFormResponse() ever need to print values for --simple-value?

I'll add a check --simple-value to CMICmdCmdVarUpdate::Acknowledge() though, to print values for when m_bValueChangedNormalType is set.

lldb/tools/lldb-mi/MICmdCmdVar.cpp
339–341 ↗(On Diff #21010)

Will do

343–357 ↗(On Diff #21010)

no problem

EwanCrawford edited edge metadata.

Updated patch with suggested changes

ki.stfu accepted this revision.Mar 3 2015, 4:10 AM
ki.stfu edited edge metadata.

lgtm

This revision is now accepted and ready to land.Mar 3 2015, 4:10 AM

Thanks Ilia. I don't have commit access, would you mind committing?

ki.stfu commandeered this revision.Mar 4 2015, 3:13 AM
ki.stfu edited reviewers, added: EwanCrawford; removed: ki.stfu.
This revision now requires review to proceed.Mar 4 2015, 3:13 AM
ki.stfu updated this revision to Diff 21186.Mar 4 2015, 3:14 AM
ki.stfu retitled this revision from Added long 'print-values' option for var-update MI command. to Added long 'print-values' option for var-update MI command..
ki.stfu updated this object.

fix diff path

ki.stfu updated this object.Mar 4 2015, 3:16 AM
This revision was automatically updated to reflect the committed changes.