[Lldb-commits] [PATCH] D50751: Allow use of self.filecheck in LLDB tests (c.f self.expect)
JDevlieghere accepted this revision. JDevlieghere added a comment. Thanks Vedant, this looks very useful! https://reviews.llvm.org/D50751 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D50751: Allow use of self.filecheck in LLDB tests (c.f self.expect)
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. LGTM minus one small detail (see inline comment). Thanks Vedant! Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:51 import sys +import tempfile import time I think we don't need that import anymore. https://reviews.llvm.org/D50751 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D49739: Add new API to SBTarget class
tatyana-krasnukha added a comment. On CentOS this test hangs forever. If I manually kill lldb-mi and lldb-server subprocesses, the output is -- Exit Code: 1 Command Output (stderr): -- /lldb/lit/tools/lldb-mi/target/target-select-so-path.test:16:10: error: CHECK: expected string not found in input # CHECK: ^done ^ :4:1: note: scanning from here (gdb) ^ :5:37: note: possible intended match here =library-loaded,id="/lldb/lit/tools/lldb-mi/target/Output/target-select-so-path.test.tmp",target-name="/lldb/lit/tools/lldb-mi/target/Output/target-select-so-path.test.tmp",host-name="/lldb/lit/tools/lldb-mi/target/Output/target-select-so-path.test.tmp",symbols-loaded="0",loaded_addr="-",size="0" Repository: rL LLVM https://reviews.llvm.org/D49739 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r342508 - Allow use of self.filecheck in LLDB tests (c.f self.expect)
Author: vedantk
Date: Tue Sep 18 12:31:47 2018
New Revision: 342508
URL: http://llvm.org/viewvc/llvm-project?rev=342508&view=rev
Log:
Allow use of self.filecheck in LLDB tests (c.f self.expect)
Add a "filecheck" method to the LLDB test base. This allows test authors
to pattern match command output using FileCheck, making it possible to
write stricter tests than what `self.expect` allows.
For context (motivation, examples of stricter checking, etc), see the
lldb-dev thread: "Using FileCheck in lldb inline tests".
Differential Revision: https://reviews.llvm.org/D50751
Modified:
lldb/trunk/CMakeLists.txt
lldb/trunk/lldb.xcodeproj/project.pbxproj
lldb/trunk/packages/Python/lldbsuite/test/configuration.py
lldb/trunk/packages/Python/lldbsuite/test/dotest.py
lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py
lldb/trunk/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/typedef_array/main.cpp
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
lldb/trunk/test/CMakeLists.txt
Modified: lldb/trunk/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/CMakeLists.txt?rev=342508&r1=342507&r2=342508&view=diff
==
--- lldb/trunk/CMakeLists.txt (original)
+++ lldb/trunk/CMakeLists.txt Tue Sep 18 12:31:47 2018
@@ -93,11 +93,15 @@ option(LLDB_TEST_USE_CUSTOM_C_COMPILER "
option(LLDB_TEST_USE_CUSTOM_CXX_COMPILER "Use the C++ compiler provided via
LLDB_TEST_CXX_COMPILER for building test inferiors (instead of the just-built
compiler). Defaults to OFF." OFF)
if(LLDB_INCLUDE_TESTS)
- # The difference between the following two paths is significant. The path to
- # LLDB will point to LLDB's binary directory, while the other will point to
- # LLVM's binary directory in case the two differ.
+ # Set the path to the default lldb test executable. Make the path relative to
+ # LLVM_RUNTIME_OUTPUT_INTDIR: this will be correct even when LLVM and LLDB
+ # have separate binary directories.
set(LLDB_DEFAULT_TEST_EXECUTABLE
"${LLVM_RUNTIME_OUTPUT_INTDIR}/lldb${CMAKE_EXECUTABLE_SUFFIX}")
+
+ # Set the paths to default llvm tools. Make these paths relative to the LLVM
+ # binary directory.
set(LLDB_DEFAULT_TEST_DSYMUTIL
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/dsymutil${CMAKE_EXECUTABLE_SUFFIX}")
+ set(LLDB_DEFAULT_TEST_FILECHECK
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/FileCheck${CMAKE_EXECUTABLE_SUFFIX}")
if (NOT LLDB_TEST_USE_CUSTOM_C_COMPILER AND TARGET clang)
set(LLDB_DEFAULT_TEST_C_COMPILER
"${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/clang${CMAKE_EXECUTABLE_SUFFIX}")
@@ -115,6 +119,7 @@ if(LLDB_INCLUDE_TESTS)
set(LLDB_TEST_C_COMPILER "${LLDB_DEFAULT_TEST_C_COMPILER}" CACHE PATH "C
Compiler to use for building LLDB test inferiors")
set(LLDB_TEST_CXX_COMPILER "${LLDB_DEFAULT_TEST_CXX_COMPILER}" CACHE PATH
"C++ Compiler to use for building LLDB test inferiors")
set(LLDB_TEST_DSYMUTIL "${LLDB_DEFAULT_TEST_DSYMUTIL}" CACHE PATH "dsymutil
used for generating dSYM bundles")
+ set(LLDB_TEST_FILECHECK "${LLDB_DEFAULT_TEST_FILECHECK}" CACHE PATH
"FileCheck used for testing purposes")
if (("${LLDB_TEST_C_COMPILER}" STREQUAL "") OR
("${LLDB_TEST_CXX_COMPILER}" STREQUAL ""))
Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=342508&r1=342507&r2=342508&view=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Sep 18 12:31:47 2018
@@ -7091,7 +7091,7 @@
/* Begin PBXLegacyTarget section */
2387551E1C24974600CCE8C3 /* lldb-python-test-suite */ = {
isa = PBXLegacyTarget;
- buildArgumentsString = "-u $(SRCROOT)/test/dotest.py
--apple-sdk $(PLATFORM_NAME)
--executable=$(BUILD_DIR)/$(CONFIGURATION)$(EFFECTIVE_PLATFORM_NAME)/lldb -C
$(LLDB_PYTHON_TESTSUITE_CC) --arch $(LLDB_PYTHON_TESTSUITE_ARCH)
--session-file-format fm --results-formatter
lldbsuite.test_event.formatter.xunit.XunitFormatter --build-dir
$(BUILD_DIR)/lldb-test-build.noindex --results-file
$(BUILD_DIR)/test-results-$(LLDB_PYTHON_TESTSUITE_ARCH).xml --rerun-all-issues
--env TERM=vt100 -O--xpass=ignore";
+ buildArgumentsString = "-u $(SRCROOT)/test/dotest.py
--apple-sdk $(PLATFORM_NAME)
--executable=$(BUILD_DIR)/$(CONFIGURATION)$(EFFECTIVE_PLATFORM_NAME)/lldb -C
$(LLDB_PYTHON_TESTSUITE_CC) --arch $(LLDB_PYTHON_TESTSUITE_ARCH) --filecheck
$(LLVM_BUILD_DIR)/x86_64/bin/FileCheck --session-file-format fm
--results-formatter lldbsuite.test_event.formatter.xunit.XunitFormatter
--build-dir $(BUILD_DIR)/lldb-test-build.noindex --results-file
$(BUILD_DIR)/test-results-$(LLDB_PYTHON_
[Lldb-commits] [PATCH] D50751: Allow use of self.filecheck in LLDB tests (c.f self.expect)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL342508: Allow use of self.filecheck in LLDB tests (c.f
self.expect) (authored by vedantk, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D50751?vs=165613&id=166018#toc
Repository:
rL LLVM
https://reviews.llvm.org/D50751
Files:
lldb/trunk/CMakeLists.txt
lldb/trunk/lldb.xcodeproj/project.pbxproj
lldb/trunk/packages/Python/lldbsuite/test/configuration.py
lldb/trunk/packages/Python/lldbsuite/test/dotest.py
lldb/trunk/packages/Python/lldbsuite/test/dotest_args.py
lldb/trunk/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/typedef_array/main.cpp
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
lldb/trunk/test/CMakeLists.txt
Index: lldb/trunk/test/CMakeLists.txt
===
--- lldb/trunk/test/CMakeLists.txt
+++ lldb/trunk/test/CMakeLists.txt
@@ -49,6 +49,7 @@
list(APPEND LLDB_TEST_COMMON_ARGS
--executable ${LLDB_TEST_EXECUTABLE}
--dsymutil ${LLDB_TEST_DSYMUTIL}
+ --filecheck ${LLDB_TEST_FILECHECK}
-C ${LLDB_TEST_C_COMPILER}
)
Index: lldb/trunk/packages/Python/lldbsuite/test/configuration.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/configuration.py
+++ lldb/trunk/packages/Python/lldbsuite/test/configuration.py
@@ -46,6 +46,9 @@
arch = None# Must be initialized after option parsing
compiler = None# Must be initialized after option parsing
+# Path to the FileCheck testing tool. Not optional.
+filecheck = None
+
# The arch might dictate some specific CFLAGS to be passed to the toolchain to build
# the inferior programs. The global variable cflags_extras provides a hook to do
# just that.
@@ -179,3 +182,11 @@
return test_subdir
return os.path.dirname(os.path.realpath(__file__))
+
+
+def get_filecheck_path():
+"""
+Get the path to the FileCheck testing tool.
+"""
+assert os.path.lexists(filecheck)
+return filecheck
Index: lldb/trunk/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
+++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
@@ -57,14 +57,21 @@
self.runCmd("frame variable foo1.b --show-types")
self.runCmd("frame variable foo1.b.b_ref --show-types")
-self.expect(
-"expression --show-types -- *(new foo(47))",
-substrs=[
-'(int) a = 47',
-'(bar) b = {',
-'(int) i = 94',
-'(baz) b = {',
-'(int) k = 99'])
+self.filecheck("expression --show-types -- *(new foo(47))", __file__,
+'-check-prefix=EXPR-TYPES-NEW-FOO')
+# EXPR-TYPES-NEW-FOO: (foo) ${{.*}} = {
+# EXPR-TYPES-NEW-FOO-NEXT: (int) a = 47
+# EXPR-TYPES-NEW-FOO-NEXT: (int *) a_ptr = 0x
+# EXPR-TYPES-NEW-FOO-NEXT: (bar) b = {
+# EXPR-TYPES-NEW-FOO-NEXT: (int) i = 94
+# EXPR-TYPES-NEW-FOO-NEXT: (int *) i_ptr = 0x
+# EXPR-TYPES-NEW-FOO-NEXT: (baz) b = {
+# EXPR-TYPES-NEW-FOO-NEXT: (int) h = 97
+# EXPR-TYPES-NEW-FOO-NEXT: (int) k = 99
+# EXPR-TYPES-NEW-FOO-NEXT: }
+# EXPR-TYPES-NEW-FOO-NEXT: (baz &) b_ref = 0x
+# EXPR-TYPES-NEW-FOO-NEXT: }
+# EXPR-TYPES-NEW-FOO-NEXT: }
self.runCmd("type summary add -F formatters.foo_SummaryProvider foo")
@@ -80,68 +87,49 @@
self.expect("expression foo1.a_ptr",
substrs=['(int *) $', '= 0x', ' -> 13'])
-self.expect(
-"expression foo1",
-substrs=[
-'(foo) $',
-' a = 12',
-'a_ptr = ',
-' -> 13',
-'i = 24',
-'i_ptr = ',
-' -> 25'])
-
-self.expect(
-"expression --ptr-depth=1 -- new foo(47)",
-substrs=[
-'(foo *) $',
-'a = 47',
-'a_ptr = ',
-' -> 48',
-'i = 94',
-'i_ptr = ',
-' -> 95'])
-
-self.expect(
-"expression foo2",
-substrs=[
-'(foo) $',
-'a = 121',
-'a_ptr = ',
-' -> 122',
-'i = 242',
-'i_ptr = ',
-' -> 243'])
+self.filecheck("expression foo1", __file__, '-check-prefix=EXPR-FOO1')
+# EXPR-FOO1: (foo) $
+# EXPR-FOO1-SAME: a = 12
[Lldb-commits] [PATCH] D50751: Allow use of self.filecheck in LLDB tests (c.f self.expect)
This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB342508: Allow use of self.filecheck in LLDB tests (c.f
self.expect) (authored by vedantk, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50751?vs=165613&id=166017#toc
Repository:
rL LLVM
https://reviews.llvm.org/D50751
Files:
CMakeLists.txt
lldb.xcodeproj/project.pbxproj
packages/Python/lldbsuite/test/configuration.py
packages/Python/lldbsuite/test/dotest.py
packages/Python/lldbsuite/test/dotest_args.py
packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
packages/Python/lldbsuite/test/functionalities/data-formatter/typedef_array/main.cpp
packages/Python/lldbsuite/test/lldbtest.py
test/CMakeLists.txt
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -49,6 +49,7 @@
list(APPEND LLDB_TEST_COMMON_ARGS
--executable ${LLDB_TEST_EXECUTABLE}
--dsymutil ${LLDB_TEST_DSYMUTIL}
+ --filecheck ${LLDB_TEST_FILECHECK}
-C ${LLDB_TEST_C_COMPILER}
)
Index: packages/Python/lldbsuite/test/configuration.py
===
--- packages/Python/lldbsuite/test/configuration.py
+++ packages/Python/lldbsuite/test/configuration.py
@@ -46,6 +46,9 @@
arch = None# Must be initialized after option parsing
compiler = None# Must be initialized after option parsing
+# Path to the FileCheck testing tool. Not optional.
+filecheck = None
+
# The arch might dictate some specific CFLAGS to be passed to the toolchain to build
# the inferior programs. The global variable cflags_extras provides a hook to do
# just that.
@@ -179,3 +182,11 @@
return test_subdir
return os.path.dirname(os.path.realpath(__file__))
+
+
+def get_filecheck_path():
+"""
+Get the path to the FileCheck testing tool.
+"""
+assert os.path.lexists(filecheck)
+return filecheck
Index: packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
===
--- packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
+++ packages/Python/lldbsuite/test/expression_command/formatters/TestFormatters.py
@@ -57,14 +57,21 @@
self.runCmd("frame variable foo1.b --show-types")
self.runCmd("frame variable foo1.b.b_ref --show-types")
-self.expect(
-"expression --show-types -- *(new foo(47))",
-substrs=[
-'(int) a = 47',
-'(bar) b = {',
-'(int) i = 94',
-'(baz) b = {',
-'(int) k = 99'])
+self.filecheck("expression --show-types -- *(new foo(47))", __file__,
+'-check-prefix=EXPR-TYPES-NEW-FOO')
+# EXPR-TYPES-NEW-FOO: (foo) ${{.*}} = {
+# EXPR-TYPES-NEW-FOO-NEXT: (int) a = 47
+# EXPR-TYPES-NEW-FOO-NEXT: (int *) a_ptr = 0x
+# EXPR-TYPES-NEW-FOO-NEXT: (bar) b = {
+# EXPR-TYPES-NEW-FOO-NEXT: (int) i = 94
+# EXPR-TYPES-NEW-FOO-NEXT: (int *) i_ptr = 0x
+# EXPR-TYPES-NEW-FOO-NEXT: (baz) b = {
+# EXPR-TYPES-NEW-FOO-NEXT: (int) h = 97
+# EXPR-TYPES-NEW-FOO-NEXT: (int) k = 99
+# EXPR-TYPES-NEW-FOO-NEXT: }
+# EXPR-TYPES-NEW-FOO-NEXT: (baz &) b_ref = 0x
+# EXPR-TYPES-NEW-FOO-NEXT: }
+# EXPR-TYPES-NEW-FOO-NEXT: }
self.runCmd("type summary add -F formatters.foo_SummaryProvider foo")
@@ -80,68 +87,49 @@
self.expect("expression foo1.a_ptr",
substrs=['(int *) $', '= 0x', ' -> 13'])
-self.expect(
-"expression foo1",
-substrs=[
-'(foo) $',
-' a = 12',
-'a_ptr = ',
-' -> 13',
-'i = 24',
-'i_ptr = ',
-' -> 25'])
-
-self.expect(
-"expression --ptr-depth=1 -- new foo(47)",
-substrs=[
-'(foo *) $',
-'a = 47',
-'a_ptr = ',
-' -> 48',
-'i = 94',
-'i_ptr = ',
-' -> 95'])
-
-self.expect(
-"expression foo2",
-substrs=[
-'(foo) $',
-'a = 121',
-'a_ptr = ',
-' -> 122',
-'i = 242',
-'i_ptr = ',
-' -> 243'])
+self.filecheck("expression foo1", __file__, '-check-prefix=EXPR-FOO1')
+# EXPR-FOO1: (foo) $
+# EXPR-FOO1-SAME: a = 12
+# EXPR-FOO1-SAME: a_ptr = {{[0-9]+}} -> 13
+# EXPR-FOO1-SAME: i = 24
+# EXPR-FOO1-SAME: i_ptr = {{[0-9]+}} -> 25
+# EXPR-FOO1-SAME: b_ref = {{[0-9]+}}
+# EXPR-FOO1-SAME: h = 27
+# EXPR-FOO
[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame
shafik created this revision.
shafik added a reviewer: jingham.
SBFrame should be a thin wrapper around the core functionality in StackFrame.
Currently FindVariable() core functionality is implemented in SBFrame and this
will move that into StackFrame.
This is step two in enabling stepping into std::function.
https://reviews.llvm.org/D52247
Files:
include/lldb/Target/StackFrame.h
source/API/SBFrame.cpp
source/Target/StackFrame.cpp
Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -1709,6 +1709,41 @@
GetFrameCodeAddress());
}
+lldb::ValueObjectSP StackFrame::FindVariable(const char *name) {
+ ValueObjectSP value_sp;
+
+ if (name == nullptr || name[0] == '\0')
+return value_sp;
+
+ TargetSP target_sp = CalculateTarget();
+ ProcessSP process_sp = CalculateProcess();
+
+ if (!target_sp && !process_sp)
+return value_sp;
+
+ VariableList variable_list;
+ VariableSP var_sp;
+ SymbolContext sc(GetSymbolContext(eSymbolContextBlock));
+
+ if (sc.block) {
+const bool can_create = true;
+const bool get_parent_variables = true;
+const bool stop_if_block_is_inlined_function = true;
+
+if (sc.block->AppendVariables(
+can_create, get_parent_variables,
stop_if_block_is_inlined_function,
+[this](Variable *v) { return v->IsInScope(this); },
+&variable_list)) {
+ var_sp = variable_list.FindVariable(ConstString(name));
+}
+
+if (var_sp)
+ value_sp = GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+ }
+
+ return value_sp;
+}
+
TargetSP StackFrame::CalculateTarget() {
TargetSP target_sp;
ThreadSP thread_sp(GetThread());
Index: source/API/SBFrame.cpp
===
--- source/API/SBFrame.cpp
+++ source/API/SBFrame.cpp
@@ -666,28 +666,10 @@
if (stop_locker.TryLock(&process->GetRunLock())) {
frame = exe_ctx.GetFramePtr();
if (frame) {
-VariableList variable_list;
-SymbolContext sc(frame->GetSymbolContext(eSymbolContextBlock));
-
-if (sc.block) {
- const bool can_create = true;
- const bool get_parent_variables = true;
- const bool stop_if_block_is_inlined_function = true;
+value_sp = frame->FindVariable(name);
- if (sc.block->AppendVariables(
- can_create, get_parent_variables,
- stop_if_block_is_inlined_function,
- [frame](Variable *v) { return v->IsInScope(frame); },
- &variable_list)) {
-var_sp = variable_list.FindVariable(ConstString(name));
- }
-}
-
-if (var_sp) {
- value_sp =
- frame->GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+if (value_sp)
sb_value.SetSP(value_sp, use_dynamic);
-}
} else {
if (log)
log->Printf("SBFrame::FindVariable () => error: could not "
Index: include/lldb/Target/StackFrame.h
===
--- include/lldb/Target/StackFrame.h
+++ include/lldb/Target/StackFrame.h
@@ -503,6 +503,8 @@
lldb::ValueObjectSP GuessValueForRegisterAndOffset(ConstString reg,
int64_t offset);
+ lldb::ValueObjectSP FindVariable(const char *name);
+
//--
// lldb::ExecutionContextScope pure virtual functions
//--
Index: source/Target/StackFrame.cpp
===
--- source/Target/StackFrame.cpp
+++ source/Target/StackFrame.cpp
@@ -1709,6 +1709,41 @@
GetFrameCodeAddress());
}
+lldb::ValueObjectSP StackFrame::FindVariable(const char *name) {
+ ValueObjectSP value_sp;
+
+ if (name == nullptr || name[0] == '\0')
+return value_sp;
+
+ TargetSP target_sp = CalculateTarget();
+ ProcessSP process_sp = CalculateProcess();
+
+ if (!target_sp && !process_sp)
+return value_sp;
+
+ VariableList variable_list;
+ VariableSP var_sp;
+ SymbolContext sc(GetSymbolContext(eSymbolContextBlock));
+
+ if (sc.block) {
+const bool can_create = true;
+const bool get_parent_variables = true;
+const bool stop_if_block_is_inlined_function = true;
+
+if (sc.block->AppendVariables(
+can_create, get_parent_variables, stop_if_block_is_inlined_function,
+[this](Variable *v) { return v->IsInScope(this); },
+&variable_list)) {
+ var_sp = variable_list.FindVariable(ConstString(name));
+}
+
+if (var_sp)
+ value_sp = GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
+ }
+
+ return value_sp;
+}
+
TargetSP StackFrame::CalculateTarget() {
TargetSP
[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
LGTM modulo minor.
Comment at: source/Target/StackFrame.cpp:1733-1738
+if (sc.block->AppendVariables(
+can_create, get_parent_variables,
stop_if_block_is_inlined_function,
+[this](Variable *v) { return v->IsInScope(this); },
+&variable_list)) {
+ var_sp = variable_list.FindVariable(ConstString(name));
+}
This is fairly unreadable IMHO. If I were you, I would hoist the lambda out.
https://reviews.llvm.org/D52247
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame
davide added inline comments. Comment at: include/lldb/Target/StackFrame.h:506-507 + lldb::ValueObjectSP FindVariable(const char *name); + //-- Also, I think you might want to add a doxygen comment here, as this function is now part of the API. https://reviews.llvm.org/D52247 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
Comment at: include/lldb/Target/StackFrame.h:506
+ lldb::ValueObjectSP FindVariable(const char *name);
+
davide wrote:
> Also, I think you might want to add a doxygen comment here, as this function
> is now part of the API.
Make this a "ConstString name" since we need that for lookup anyway? We
couldn't do that in SBFrame cause we don't expose our constant strings outside
the API, but internally we can use ConstString
Comment at: source/API/SBFrame.cpp:669
if (frame) {
-VariableList variable_list;
-SymbolContext sc(frame->GetSymbolContext(eSymbolContextBlock));
-
-if (sc.block) {
- const bool can_create = true;
- const bool get_parent_variables = true;
- const bool stop_if_block_is_inlined_function = true;
+value_sp = frame->FindVariable(name);
```
value_sp = frame->FindVariable(ConstString(name));
```
Comment at: source/Target/StackFrame.cpp:1712
+lldb::ValueObjectSP StackFrame::FindVariable(const char *name) {
+ ValueObjectSP value_sp;
```
lldb::ValueObjectSP StackFrame::FindVariable(ConstString name) {
```
Comment at: source/Target/StackFrame.cpp:1737
+&variable_list)) {
+ var_sp = variable_list.FindVariable(ConstString(name));
+}
remove ConstString if we change parameter
https://reviews.llvm.org/D52247
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D51999: build: add libedit to include paths
compnerd requested changes to this revision. compnerd added a comment. This revision now requires changes to proceed. Can you use `target_include_directories` instead and do this only on `lldbHost` instead please? That restricts the inclusion to just that target, which would help prevent accidental inclusion of the headers. That is the change should be to `source/Host/CMakeLists.txt`. Repository: rLLDB LLDB https://reviews.llvm.org/D51999 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52247: Refactor FindVariable() core functionality into StackFrame out of SBFrame
jingham added a comment. I agree with Greg, this is the sort of function that we'd pass a ConstString to on the lldb_private side. We haven't documented all the API's we should. This one is pretty self-explanatory, but still we should try to document all the new functions we add, Davide's right there. Other than those nits, this seems fine. https://reviews.llvm.org/D52247 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
