[Lldb-commits] [PATCH] D47410: Don't include headers from inside a namespace in MIUtilSingletonHelper.h
ki.stfu added a comment. 👍 Repository: rL LLVM https://reviews.llvm.org/D47410 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31073: Enable lldm-mi commands -stack-list-locals -stack-list-variables and -var-create to work only with variables in scope
ki.stfu requested changes to this revision. ki.stfu added a comment. This revision now requires changes to proceed. Hi. LGTM. Just fix a few minor issues before committing. Thank you for your contribution to LLDB project! Comment at: packages/Python/lldbsuite/test/tools/lldb-mi/lexical-scope/Makefile:1 +LEVEL = ../../../make + Rename directory to lexical**_**scope. Comment at: packages/Python/lldbsuite/test/tools/lldb-mi/lexical-scope/TestLexicalScope.py:1 +# coding=utf8 +""" Remove please. We don't specify encoding in other files. Comment at: packages/Python/lldbsuite/test/tools/lldb-mi/lexical-scope/TestLexicalScope.py:1 +# coding=utf8 +""" ki.stfu wrote: > Remove please. We don't specify encoding in other files. Rename this to packages/Python/lldbsuite/test/tools/lldb-mi/lexical**_**scope/Test**Mi**LexicalScope.py. https://reviews.llvm.org/D31073 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D31073: Enable lldm-mi commands -stack-list-locals -stack-list-variables and -var-create to work only with variables in scope
ki.stfu accepted this revision. ki.stfu added a comment. This revision is now accepted and ready to land. Would you like me to commit it? https://reviews.llvm.org/D31073 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32340: [LLDB][MIPS] Fix TestMiExec.py failure
ki.stfu requested changes to this revision.
ki.stfu added a comment.
This revision now requires changes to proceed.
Thanks for catching this! Could you update this CL to let me commit it?
Comment at:
packages/Python/lldbsuite/test/tools/lldb-mi/control/TestMiExec.py:287-289
+self.g_MyFunction_line_no = line_number('main.cpp', "function_line_1")
+self.s_MyFunction_line_no = line_number('main.cpp', "function_line_2")
+
Remove this.
Comment at:
packages/Python/lldbsuite/test/tools/lldb-mi/control/TestMiExec.py:306-313
it =
self.expect(["\*stopped,reason=\"end-stepping-range\".+?func=\"main\"",
"\*stopped,reason=\"end-stepping-range\".+?func=\"(?!main).+?\""])
# Exit from printf if needed
if it == 1:
self.runCmd("-exec-finish")
self.expect("\^running")
self.expect(
Please do like this.
Comment at:
packages/Python/lldbsuite/test/tools/lldb-mi/control/TestMiExec.py:325-336
self.expect(
-
"\*stopped,reason=\"end-stepping-range\".+?main\.cpp\",line=\"30\"")
+
"\*stopped,reason=\"end-stepping-range\".+?main\.cpp\",line=\"(30|29)\"")
+
+finish_status = self.child.after
+string_to_match = 'line="{0}"'.format(self.g_MyFunction_line_no)
+
+# Call to s_MyFunction may not follow immediately after g_MyFunction.
```
it =
self.expect(["\*stopped,reason=\"end-stepping-range\".+?main\.cpp\",line=\"30\"",
"\*stopped,reason=\"end-stepping-range\".+?main\.cpp\",line=\"29\""])
if it == 1:
# Call to s_MyFunction may not follow immediately after g_MyFunction.
There might be
# some instructions in between to restore caller-saved registers.
self.runCmd("-exec-step")
self.expect("\^running")
self.expect(
"\*stopped,reason=\"end-stepping-range\".+?main\.cpp\",line=\"30\"")
```
Comment at: packages/Python/lldbsuite/test/tools/lldb-mi/control/main.cpp:29-30
printf("start");
-g_MyFunction();
-s_MyFunction();
+g_MyFunction(); // function_line_1
+s_MyFunction(); // function_line_2
printf("exit");
Revert this
https://reviews.llvm.org/D32340
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32340: [LLDB][MIPS] Fix TestMiExec.py failure
ki.stfu added inline comments.
Comment at:
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-mi/control/TestMiExec.py:329
+# We need to get past these instructions with a step to reach call
to s_MyFunction.
+self.runCmd("-exec-step --thread 1")
I mistyped there should be -exec-next,, and why you are not checking that it
has stopped at the right line (as I suggested)?
Repository:
rL LLVM
https://reviews.llvm.org/D32340
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D37533: Fix lldb-mi test data_read_memory_bytes_global
ki.stfu accepted this revision. ki.stfu added a comment. This revision is now accepted and ready to land. 👍 https://reviews.llvm.org/D37533 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D26124: [LLDB-MI] Escape MI output in a more consistent manner
ki.stfu requested changes to this revision.
ki.stfu added a comment.
This revision now requires changes to proceed.
@enlight , thanks for giving a chance to review it, and sorry that it took to
long from my side.
Most of these changes look good to me, especially removing of meaningless
temporal variables during construction of Result values. But I can't agree with
that:
> I've also removed the spacing code from CMICmnMIValueTuple and
> CMICmnMIValueResult, it was only misused to format composite values in
> CMICmnLLDBUtilSBValue::GetCompositeValue(). The format of composite values
> isn't specified by the GDB-MI spec, and as such these values shouldn't be
> built using CMICmnMIValueTuple, CMICmnMIValueResult, and CMICmnMIValueConst.
I'm not sure about extra spaces in `tuple` or `result` records (`vbUseSpacing`
opt in `CMICmnMIValueResult` and `CMICmnMIValueTuple` respectively), but
regarding `CMICmnLLDBUtilSBValue::GetCompositeValue` I'd like to leave "as is".
Construction of tuples by hands (see my inline comment) looks terrible for me
while using of `CMICmnMIValueTuple` looked very naturally.
Regarding your note about the format of composite values, the[[
https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Output-Syntax.html#GDB_002fMI-Output-Syntax
| GDB-MI spec ]] clearly says that:
result ==> variable "=" value
variable ==> string
value ==> const | tuple | list
const ==> c-string
tuple ==> "{}" | "{" result ( "," result )* "}"
So, the aggregates are those whose root values are represented by tuples.
Comment at: tools/lldb-mi/MICmnLLDBUtilSBValue.cpp:197-208
+ if (vwrCompositeValue.back() != '{')
+vwrCompositeValue += ", ";
+ vwrCompositeValue += CMIUtilString::Format("%s = %s",
+ utilMember.GetName().c_str(),
+ value.c_str());
} else {
+ if (vwrCompositeValue.back() != '{')
here
Repository:
rL LLVM
https://reviews.llvm.org/D26124
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D24711: [lldb-mi] Fix implementation for a few mi commands
ki.stfu added a comment. lgtm Repository: rL LLVM https://reviews.llvm.org/D24711 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29256: Do not pass non-POD type variables through variadic function
ki.stfu accepted this revision. ki.stfu added a comment. This revision is now accepted and ready to land. I don't know the point of this patch (probably it's something special for NetBSD? @emaste) but I'm okay with that. Repository: rL LLVM https://reviews.llvm.org/D29256 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29256: Do not pass non-POD type variables through variadic function
ki.stfu added a comment.
In https://reviews.llvm.org/D29256#660159, @krytarowski wrote:
> It's undefined (implementation defined) behavior.
>
> C++11 5.2.2/7:
>
>
> > Passing a potentially-evaluated argument of class type having a
> > non-trivial copy constructor, a non-trivial move contructor, or a
> > non-trivial destructor, with no corresponding parameter, is
> > conditionally-supported with implementation-defined semantics.
> >
Interesting. Passing to what? I thought it means we shouldn't pass non-trivial
types through variadic arguments (`...` expression), and in this case we don't
do it because `CMIUtilString` is the type of the parameter `vFormating` which
has a name.
Unfortunately I can't reproduce it on my Ubuntu using the following example:
#include
#include
struct Count {
int value;
explicit Count(int v) : value(v) { }
// Make it non-trivial
Count(const Count&) : value(0) { }
Count(Count&&) : value(0) { }
~Count() { if (value) value |= 1; }
};
int add_nums(Count count, ...)
{
int result = 0;
va_list args;
va_start(args, count);
for (int i = 0; i < count.value; ++i) {
result += va_arg(args, int);
}
va_end(args);
return result;
}
int main()
{
std::cout << add_nums(Count(4), 25, 25, 50, 50) << '\n';
return 0;
}
> This patch was created to address compiler warning.
Could you tell me what compiler and platform do you use and provide the exact
warning message?
Repository:
rL LLVM
https://reviews.llvm.org/D29256
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29256: Do not pass non-POD type variables through variadic function
ki.stfu added a comment. In https://reviews.llvm.org/D29256#662167, @krytarowski wrote: > @ki.stfu do you still agree with this patch? Please follow my comments and then we can go ahead. Comment at: tools/lldb-mi/MIDriver.cpp:512 CMICmnThreadMgrStd::Instance().GetErrorDescription().c_str()); -SetErrorDescriptionn(errMsg); +SetErrorDescriptionn(errMsg.c_str()); return MIstatus::failure; `SetErrorDescription` accepts a const reference to `CMIUtilString` so it suits better than `SetErrorDescriptionn` (double `n`). ``` SetErrorDescription(errMsg); ``` //Oh, certainly it should be renamed// Repository: rL LLVM https://reviews.llvm.org/D29256 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D29513: Add -gdb-set and -gdb-show support for non-stop to lldb-mi
ki.stfu accepted this revision.
ki.stfu added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: tools/lldb-mi/MICmdCmdGdbShow.cpp:350
lldb::SBDebugger &rDbgr = m_rLLDBDebugSessionInfo.GetDebugger();
- m_strValue =
lldb::SBDebugger::GetInternalVariableValue("target.x86-disassembly-flavor",
-
rDbgr.GetInstanceName()).GetStringAtIndex(0);
+ m_strValue = lldb::SBDebugger::GetInternalVariableValue(
+ "target.x86-disassembly-flavor", rDbgr.GetInstanceName())
I think clang-format will format it the different way.
https://reviews.llvm.org/D29513
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
