[Lldb-commits] [PATCH] D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests
tberghammer created this revision.
tberghammer added a reviewer: clayborg.
tberghammer added a subscriber: lldb-commits.
Herald added a subscriber: mgorny.
Improve Type::GetTypeScopeAndBasenameHelper and add unit tests
Previously it failed to handle nested types inside templated classes
making it impossible to look up these types using the fully qualified
name.
https://reviews.llvm.org/D28466
Files:
source/Symbol/Type.cpp
unittests/Symbol/CMakeLists.txt
unittests/Symbol/TestType.cpp
Index: unittests/Symbol/TestType.cpp
===
--- /dev/null
+++ unittests/Symbol/TestType.cpp
@@ -0,0 +1,51 @@
+//===-- TestType.cpp *- C++ -*-===//
+//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Symbol/Type.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+void TestGetTypeScopeAndBasenameHelper(const char *full_type,
+ bool expected_is_scoped,
+ const char *expected_scope,
+ const char *expected_name) {
+ std::string scope, name;
+ lldb::TypeClass type_class;
+ bool is_scoped =
+ Type::GetTypeScopeAndBasename(full_type, scope, name, type_class);
+ EXPECT_EQ(is_scoped, expected_is_scoped);
+ if (expected_is_scoped) {
+EXPECT_EQ(scope, expected_scope);
+EXPECT_EQ(name, expected_name);
+ }
+}
+};
+
+TEST(Type, GetTypeScopeAndBasename) {
+ TestGetTypeScopeAndBasenameHelper("int", false, "", "");
+ TestGetTypeScopeAndBasenameHelper("std::string", true, "std::", "string");
+ TestGetTypeScopeAndBasenameHelper("std::set", true, "std::", "set");
+ TestGetTypeScopeAndBasenameHelper("std::set>", true,
+"std::", "set>");
+ TestGetTypeScopeAndBasenameHelper("std::string::iterator", true,
+"std::string::", "iterator");
+ TestGetTypeScopeAndBasenameHelper("std::set::iterator", true,
+"std::set::", "iterator");
+ TestGetTypeScopeAndBasenameHelper(
+ "std::set>::iterator", true,
+ "std::set>::", "iterator");
+ TestGetTypeScopeAndBasenameHelper(
+ "std::set>::iterator", true,
+ "std::set>::", "iterator");
+}
Index: unittests/Symbol/CMakeLists.txt
===
--- unittests/Symbol/CMakeLists.txt
+++ unittests/Symbol/CMakeLists.txt
@@ -1,3 +1,4 @@
add_lldb_unittest(SymbolTests
TestClangASTContext.cpp
+ TestType.cpp
)
Index: source/Symbol/Type.cpp
===
--- source/Symbol/Type.cpp
+++ source/Symbol/Type.cpp
@@ -623,47 +623,62 @@
bool Type::GetTypeScopeAndBasename(const char *&name_cstr, std::string &scope,
std::string &basename,
TypeClass &type_class) {
+ type_class = eTypeClassAny;
+
// Protect against null c string.
+ if (!name_cstr || !name_cstr[0])
+return false;
- type_class = eTypeClassAny;
+ llvm::StringRef name_strref(name_cstr);
+ if (name_strref.startswith("struct ")) {
+name_cstr += 7;
+type_class = eTypeClassStruct;
+ } else if (name_strref.startswith("class ")) {
+name_cstr += 6;
+type_class = eTypeClassClass;
+ } else if (name_strref.startswith("union ")) {
+name_cstr += 6;
+type_class = eTypeClassUnion;
+ } else if (name_strref.startswith("enum ")) {
+name_cstr += 5;
+type_class = eTypeClassEnumeration;
+ } else if (name_strref.startswith("typedef ")) {
+name_cstr += 8;
+type_class = eTypeClassTypedef;
+ }
- if (name_cstr && name_cstr[0]) {
-llvm::StringRef name_strref(name_cstr);
-if (name_strref.startswith("struct ")) {
- name_cstr += 7;
- type_class = eTypeClassStruct;
-} else if (name_strref.startswith("class ")) {
- name_cstr += 6;
- type_class = eTypeClassClass;
-} else if (name_strref.startswith("union ")) {
- name_cstr += 6;
- type_class = eTypeClassUnion;
-} else if (name_strref.startswith("enum ")) {
- name_cstr += 5;
- type_class = eTypeClassEnumeration;
-} else if (name_strref.startswith("typedef ")) {
- name_cstr += 8;
- type_class = eTypeClassTypedef;
-}
-const char *basename_cstr = name_cstr;
-const char *namespace_separator = ::strstr(basename_cstr, "::");
-if (namespace_separator) {
- const char *template_arg_char = ::strchr(basename_cstr, '<');
- while (namespace_separator != nullptr) {
-if (template_arg_char &&
-namespace_separator > template_arg_char) // but namespace
[Lldb-commits] [PATCH] D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Switch over to using llvm::StringRef in the functions where mentioned.
Comment at: source/Symbol/Type.cpp:623
bool Type::GetTypeScopeAndBasename(const char *&name_cstr, std::string &scope,
std::string &basename,
We should probably switch over to using "llvm::StringRef &name" here.
Comment at: source/Symbol/Type.cpp:629
// Protect against null c string.
+ if (!name_cstr || !name_cstr[0])
+return false;
use name.empty() from llvm::StringRef
Comment at: source/Symbol/Type.cpp:632
- type_class = eTypeClassAny;
+ llvm::StringRef name_strref(name_cstr);
+ if (name_strref.startswith("struct ")) {
Remove and just use "llvm::StringRef &name" variable.
Comment at: source/Symbol/Type.cpp:633-634
+ llvm::StringRef name_strref(name_cstr);
+ if (name_strref.startswith("struct ")) {
+name_cstr += 7;
+type_class = eTypeClassStruct;
Change to use llvm::StringRef::consume_front and remove the "name_cstr += 7":
```
if (name.consume_front("struct ")) {
type_class = eTypeClassStruct;
} else if (...) {
```
Comment at: source/Symbol/Type.cpp:636
+type_class = eTypeClassStruct;
+ } else if (name_strref.startswith("class ")) {
+name_cstr += 6;
ditto
Comment at: source/Symbol/Type.cpp:639
+type_class = eTypeClassClass;
+ } else if (name_strref.startswith("union ")) {
+name_cstr += 6;
ditto
Comment at: source/Symbol/Type.cpp:642
+type_class = eTypeClassUnion;
+ } else if (name_strref.startswith("enum ")) {
+name_cstr += 5;
ditto
Comment at: source/Symbol/Type.cpp:645
+type_class = eTypeClassEnumeration;
+ } else if (name_strref.startswith("typedef ")) {
+name_cstr += 8;
ditto
Comment at: source/Symbol/Type.cpp:650-651
- if (name_cstr && name_cstr[0]) {
-llvm::StringRef name_strref(name_cstr);
-if (name_strref.startswith("struct ")) {
- name_cstr += 7;
- type_class = eTypeClassStruct;
-} else if (name_strref.startswith("class ")) {
- name_cstr += 6;
- type_class = eTypeClassClass;
-} else if (name_strref.startswith("union ")) {
- name_cstr += 6;
- type_class = eTypeClassUnion;
-} else if (name_strref.startswith("enum ")) {
- name_cstr += 5;
- type_class = eTypeClassEnumeration;
-} else if (name_strref.startswith("typedef ")) {
- name_cstr += 8;
- type_class = eTypeClassTypedef;
-}
-const char *basename_cstr = name_cstr;
-const char *namespace_separator = ::strstr(basename_cstr, "::");
-if (namespace_separator) {
- const char *template_arg_char = ::strchr(basename_cstr, '<');
- while (namespace_separator != nullptr) {
-if (template_arg_char &&
-namespace_separator > template_arg_char) // but namespace'd
template
- // arguments are still
good
- // to go
- break;
-basename_cstr = namespace_separator + 2;
-namespace_separator = strstr(basename_cstr, "::");
- }
- if (basename_cstr > name_cstr) {
-scope.assign(name_cstr, basename_cstr - name_cstr);
-basename.assign(basename_cstr);
-return true;
+ const char *basename_cstr = name_cstr;
+ const char *namespace_separator = ::strstr(basename_cstr, "::");
+ const char *template_arg_char = ::strchr(basename_cstr, '<');
Use llvm::StringRef and its member functions instead of strstr and starchy
below.
https://reviews.llvm.org/D28466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D28155: Force the installation of lldb-server and lldb-argdumper
beanz added a comment. I think I fixed this in https://reviews.llvm.org/rL290934. https://reviews.llvm.org/D28155 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests
tberghammer updated this revision to Diff 83669.
tberghammer marked 9 inline comments as done.
https://reviews.llvm.org/D28466
Files:
include/lldb/Symbol/Type.h
source/Core/Module.cpp
source/Symbol/Type.cpp
source/Symbol/TypeList.cpp
source/Symbol/TypeMap.cpp
unittests/Symbol/CMakeLists.txt
unittests/Symbol/TestType.cpp
Index: unittests/Symbol/TestType.cpp
===
--- /dev/null
+++ unittests/Symbol/TestType.cpp
@@ -0,0 +1,51 @@
+//===-- TestType.cpp *- C++ -*-===//
+//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Symbol/Type.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+void TestGetTypeScopeAndBasenameHelper(const char *full_type,
+ bool expected_is_scoped,
+ const char *expected_scope,
+ const char *expected_name) {
+ llvm::StringRef scope, name;
+ lldb::TypeClass type_class;
+ bool is_scoped =
+ Type::GetTypeScopeAndBasename(full_type, scope, name, type_class);
+ EXPECT_EQ(is_scoped, expected_is_scoped);
+ if (expected_is_scoped) {
+EXPECT_EQ(scope, expected_scope);
+EXPECT_EQ(name, expected_name);
+ }
+}
+};
+
+TEST(Type, GetTypeScopeAndBasename) {
+ TestGetTypeScopeAndBasenameHelper("int", false, "", "");
+ TestGetTypeScopeAndBasenameHelper("std::string", true, "std::", "string");
+ TestGetTypeScopeAndBasenameHelper("std::set", true, "std::", "set");
+ TestGetTypeScopeAndBasenameHelper("std::set>", true,
+"std::", "set>");
+ TestGetTypeScopeAndBasenameHelper("std::string::iterator", true,
+"std::string::", "iterator");
+ TestGetTypeScopeAndBasenameHelper("std::set::iterator", true,
+"std::set::", "iterator");
+ TestGetTypeScopeAndBasenameHelper(
+ "std::set>::iterator", true,
+ "std::set>::", "iterator");
+ TestGetTypeScopeAndBasenameHelper(
+ "std::set>::iterator", true,
+ "std::set>::", "iterator");
+}
Index: unittests/Symbol/CMakeLists.txt
===
--- unittests/Symbol/CMakeLists.txt
+++ unittests/Symbol/CMakeLists.txt
@@ -1,3 +1,4 @@
add_lldb_unittest(SymbolTests
TestClangASTContext.cpp
+ TestType.cpp
)
Index: source/Symbol/TypeMap.cpp
===
--- source/Symbol/TypeMap.cpp
+++ source/Symbol/TypeMap.cpp
@@ -152,13 +152,13 @@
void TypeMap::RemoveMismatchedTypes(const char *qualified_typename,
bool exact_match) {
- std::string type_scope;
- std::string type_basename;
+ llvm::StringRef type_scope;
+ llvm::StringRef type_basename;
TypeClass type_class = eTypeClassAny;
if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope,
type_basename, type_class)) {
type_basename = qualified_typename;
-type_scope.clear();
+type_scope = "";
}
return RemoveMismatchedTypes(type_scope, type_basename, type_class,
exact_match);
@@ -189,8 +189,8 @@
ConstString match_type_name_const_str(the_type->GetQualifiedName());
if (match_type_name_const_str) {
const char *match_type_name = match_type_name_const_str.GetCString();
- std::string match_type_scope;
- std::string match_type_basename;
+ llvm::StringRef match_type_scope;
+ llvm::StringRef match_type_basename;
if (Type::GetTypeScopeAndBasename(match_type_name, match_type_scope,
match_type_basename,
match_type_class)) {
Index: source/Symbol/TypeList.cpp
===
--- source/Symbol/TypeList.cpp
+++ source/Symbol/TypeList.cpp
@@ -108,13 +108,13 @@
void TypeList::RemoveMismatchedTypes(const char *qualified_typename,
bool exact_match) {
- std::string type_scope;
- std::string type_basename;
+ llvm::StringRef type_scope;
+ llvm::StringRef type_basename;
TypeClass type_class = eTypeClassAny;
if (!Type::GetTypeScopeAndBasename(qualified_typename, type_scope,
type_basename, type_class)) {
type_basename = qualified_typename;
-type_scope.clear();
+type_scope = "";
}
return RemoveMismatchedTypes(type_scope, type_basename, type_class,
exact_match);
@@ -145,8 +145,8 @@
ConstString match_type_name_const_str(the_type->GetQual
[Lldb-commits] [PATCH] D28466: Improve Type::GetTypeScopeAndBasenameHelper and add unit tests
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Fix the consume_front mentioned in the inlined comment and this is good to go.
Comment at: source/Core/Module.cpp:1010-1011
if (type_scope.size() >= 2 && type_scope[0] == ':' &&
type_scope[1] == ':') {
+ type_scope = type_scope.drop_back(2);
Use consume_front here:
```
if (type_scope.consume_front("::"))
exact_match = true;
```
https://reviews.llvm.org/D28466
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r291500 - [cmake] Obtain LLVM_CMAKE_PATH from llvm-config
Author: mgorny
Date: Mon Jan 9 17:12:37 2017
New Revision: 291500
URL: http://llvm.org/viewvc/llvm-project?rev=291500&view=rev
Log:
[cmake] Obtain LLVM_CMAKE_PATH from llvm-config
Use the new --cmakedir option to obtain LLVM_CMAKE_PATH straight from
llvm-config instead of reconstructing it locally.
Modified:
lldb/trunk/cmake/modules/LLDBStandalone.cmake
Modified: lldb/trunk/cmake/modules/LLDBStandalone.cmake
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBStandalone.cmake?rev=291500&r1=291499&r2=291500&view=diff
==
--- lldb/trunk/cmake/modules/LLDBStandalone.cmake (original)
+++ lldb/trunk/cmake/modules/LLDBStandalone.cmake Mon Jan 9 17:12:37 2017
@@ -21,7 +21,8 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
"--libdir"
"--includedir"
"--prefix"
- "--src-root")
+ "--src-root"
+ "--cmakedir")
execute_process(
COMMAND ${CONFIG_COMMAND}
RESULT_VARIABLE HAD_ERROR
@@ -47,6 +48,7 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
list(GET CONFIG_OUTPUT 3 INCLUDE_DIR)
list(GET CONFIG_OUTPUT 4 LLVM_OBJ_ROOT)
list(GET CONFIG_OUTPUT 5 MAIN_SRC_DIR)
+ list(GET CONFIG_OUTPUT 6 LLVM_CMAKE_PATH)
if(NOT MSVC_IDE)
set(LLVM_ENABLE_ASSERTIONS ${ENABLE_ASSERTIONS}
@@ -65,7 +67,6 @@ if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURR
find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
NO_DEFAULT_PATH)
- set(LLVM_CMAKE_PATH "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
set(LLVMCONFIG_FILE "${LLVM_CMAKE_PATH}/LLVMConfig.cmake")
if(EXISTS ${LLVMCONFIG_FILE})
list(APPEND CMAKE_MODULE_PATH "${LLVM_CMAKE_PATH}")
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r291522 - Stop limiting the number of TSan backtrace size to 8
Author: kuba.brecka
Date: Mon Jan 9 19:14:52 2017
New Revision: 291522
URL: http://llvm.org/viewvc/llvm-project?rev=291522&view=rev
Log:
Stop limiting the number of TSan backtrace size to 8
We currently limit the length of TSan returned backtraces to 8, which is not
necessary (and a bug, most likely). Let's remove this.
Differential Revision: https://reviews.llvm.org/D28035
Modified:
lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
Modified:
lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp?rev=291522&r1=291521&r2=291522&view=diff
==
---
lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
(original)
+++
lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
Mon Jan 9 19:14:52 2017
@@ -206,7 +206,8 @@ CreateStackTrace(ValueObjectSP o,
StructuredData::Array *trace = new StructuredData::Array();
ValueObjectSP trace_value_object =
o->GetValueForExpressionPath(trace_item_name.c_str());
- for (int j = 0; j < 8; j++) {
+ size_t count = trace_value_object->GetNumChildren();
+ for (size_t j = 0; j < count; j++) {
addr_t trace_addr =
trace_value_object->GetChildAtIndex(j, true)->GetValueAsUnsigned(0);
if (trace_addr == 0)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D28035: Stop limiting the number of TSan backtrace size to 8
This revision was automatically updated to reflect the committed changes.
Closed by commit rL291522: Stop limiting the number of TSan backtrace size to 8
(authored by kuba.brecka).
Changed prior to commit:
https://reviews.llvm.org/D28035?vs=82280&id=83753#toc
Repository:
rL LLVM
https://reviews.llvm.org/D28035
Files:
lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
Index:
lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
===
---
lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
+++
lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
@@ -206,7 +206,8 @@
StructuredData::Array *trace = new StructuredData::Array();
ValueObjectSP trace_value_object =
o->GetValueForExpressionPath(trace_item_name.c_str());
- for (int j = 0; j < 8; j++) {
+ size_t count = trace_value_object->GetNumChildren();
+ for (size_t j = 0; j < count; j++) {
addr_t trace_addr =
trace_value_object->GetChildAtIndex(j, true)->GetValueAsUnsigned(0);
if (trace_addr == 0)
Index: lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
===
--- lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
+++ lldb/trunk/source/Plugins/InstrumentationRuntime/ThreadSanitizer/ThreadSanitizerRuntime.cpp
@@ -206,7 +206,8 @@
StructuredData::Array *trace = new StructuredData::Array();
ValueObjectSP trace_value_object =
o->GetValueForExpressionPath(trace_item_name.c_str());
- for (int j = 0; j < 8; j++) {
+ size_t count = trace_value_object->GetNumChildren();
+ for (size_t j = 0; j < count; j++) {
addr_t trace_addr =
trace_value_object->GetChildAtIndex(j, true)->GetValueAsUnsigned(0);
if (trace_addr == 0)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
