Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
labath added a comment.
Thank you for writing the tests. I have two stylistic comments, but otherwise
looks great.
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:113
@@ -105,4 +112,3 @@
case 'J':
-// Asynchronous JSON packet, destined for a
-// StructuredDataPlugin.
{
+// Verify this J packet is a JSON-async: packet.
I'd like to move this code to a separate function. The main job of
`SendContinuePacketAndWaitForResponse` is dealing with the thread
synchronization issues, which is tricky enough without having json parsing in
the middle of it.
Comment at: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp:371
@@ +370,3 @@
+ASSERT_EQ("T01", response.GetStringRef());
+ASSERT_EQ(fix.delegate.structured_data_entries.size(), 1);
+
Please put the "expected" values first (`ASSERT_NE(expected, actual)`).
Otherwise the error message will come out wrong when the comparison fails. The
same issue is present in a number of other checks.
https://reviews.llvm.org/D23884
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23882: Replace uses of MIUtilParse::CRegexParser with llvm::Regex
labath added a subscriber: labath. labath added reviewers: ki.stfu, abidh. labath added a comment. Woohoo. I am glad that this is finally getting fixed. https://reviews.llvm.org/D23882 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r279808 - Add cmake option to choose whether to use the builtin demangler
Author: labath Date: Fri Aug 26 04:47:58 2016 New Revision: 279808 URL: http://llvm.org/viewvc/llvm-project?rev=279808&view=rev Log: Add cmake option to choose whether to use the builtin demangler Summary: Previously the builting demangler was on for platforms that explicitly set a flag by modifying Mangled.cpp (windows, freebsd). The Xcode build always used builtin demangler by passing a compiler flag. This adds a cmake flag (defaulting to ON) to configure the demangling library used at build time. The flag is only available on non-windows platforms as there the system demangler is not present (in the form we're trying to use it, at least). The impact of this change is: - linux: switches to the builtin demangler - freebsd, windows: NFC (I hope) - netbsd: switches to the builtin demangler - osx cmake build: switches to the builtin demangler (matching the XCode build) The main motivation for this is the cross-platform case, where it should bring more consistency by removing the dependency on the host demangler (which can be completely unrelated to the debug target). Reviewers: zturner, emaste, krytarowski Subscribers: emaste, clayborg, lldb-commits Differential Revision: https://reviews.llvm.org/D23830 Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake lldb/trunk/source/Core/Mangled.cpp Modified: lldb/trunk/cmake/modules/LLDBConfig.cmake URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/LLDBConfig.cmake?rev=279808&r1=279807&r2=279808&view=diff == --- lldb/trunk/cmake/modules/LLDBConfig.cmake (original) +++ lldb/trunk/cmake/modules/LLDBConfig.cmake Fri Aug 26 04:47:58 2016 @@ -401,3 +401,12 @@ if(LLDB_USING_LIBSTDCXX) "- ignore this warning and accept occasional instability") endif() endif() + +if(MSVC) +set(LLDB_USE_BUILTIN_DEMANGLER ON) +else() +option(LLDB_USE_BUILTIN_DEMANGLER "Use lldb's builtin demangler instead of the system one" ON) +endif() +if(LLDB_USE_BUILTIN_DEMANGLER) +add_definitions(-DLLDB_USE_BUILTIN_DEMANGLER) +endif() Modified: lldb/trunk/source/Core/Mangled.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Mangled.cpp?rev=279808&r1=279807&r2=279808&view=diff == --- lldb/trunk/source/Core/Mangled.cpp (original) +++ lldb/trunk/source/Core/Mangled.cpp Fri Aug 26 04:47:58 2016 @@ -14,20 +14,15 @@ #include "lldb/Host/windows/windows.h" #include #pragma comment(lib, "dbghelp.lib") -#define LLDB_USE_BUILTIN_DEMANGLER -#elif defined (__FreeBSD__) -#define LLDB_USE_BUILTIN_DEMANGLER -#else -#include #endif #ifdef LLDB_USE_BUILTIN_DEMANGLER - // Provide a fast-path demangler implemented in FastDemangle.cpp until it can // replace the existing C++ demangler with a complete implementation #include "lldb/Core/FastDemangle.h" #include "lldb/Core/CxaDemangle.h" - +#else +#include #endif ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23830: Add cmake option to choose whether to use the builtin demangler
This revision was automatically updated to reflect the committed changes. Closed by commit rL279808: Add cmake option to choose whether to use the builtin demangler (authored by labath). Changed prior to commit: https://reviews.llvm.org/D23830?vs=69093&id=69333#toc Repository: rL LLVM https://reviews.llvm.org/D23830 Files: lldb/trunk/cmake/modules/LLDBConfig.cmake lldb/trunk/source/Core/Mangled.cpp Index: lldb/trunk/source/Core/Mangled.cpp === --- lldb/trunk/source/Core/Mangled.cpp +++ lldb/trunk/source/Core/Mangled.cpp @@ -14,20 +14,15 @@ #include "lldb/Host/windows/windows.h" #include #pragma comment(lib, "dbghelp.lib") -#define LLDB_USE_BUILTIN_DEMANGLER -#elif defined (__FreeBSD__) -#define LLDB_USE_BUILTIN_DEMANGLER -#else -#include #endif #ifdef LLDB_USE_BUILTIN_DEMANGLER - // Provide a fast-path demangler implemented in FastDemangle.cpp until it can // replace the existing C++ demangler with a complete implementation #include "lldb/Core/FastDemangle.h" #include "lldb/Core/CxaDemangle.h" - +#else +#include #endif Index: lldb/trunk/cmake/modules/LLDBConfig.cmake === --- lldb/trunk/cmake/modules/LLDBConfig.cmake +++ lldb/trunk/cmake/modules/LLDBConfig.cmake @@ -401,3 +401,12 @@ "- ignore this warning and accept occasional instability") endif() endif() + +if(MSVC) +set(LLDB_USE_BUILTIN_DEMANGLER ON) +else() +option(LLDB_USE_BUILTIN_DEMANGLER "Use lldb's builtin demangler instead of the system one" ON) +endif() +if(LLDB_USE_BUILTIN_DEMANGLER) +add_definitions(-DLLDB_USE_BUILTIN_DEMANGLER) +endif() Index: lldb/trunk/source/Core/Mangled.cpp === --- lldb/trunk/source/Core/Mangled.cpp +++ lldb/trunk/source/Core/Mangled.cpp @@ -14,20 +14,15 @@ #include "lldb/Host/windows/windows.h" #include #pragma comment(lib, "dbghelp.lib") -#define LLDB_USE_BUILTIN_DEMANGLER -#elif defined (__FreeBSD__) -#define LLDB_USE_BUILTIN_DEMANGLER -#else -#include #endif #ifdef LLDB_USE_BUILTIN_DEMANGLER - // Provide a fast-path demangler implemented in FastDemangle.cpp until it can // replace the existing C++ demangler with a complete implementation #include "lldb/Core/FastDemangle.h" #include "lldb/Core/CxaDemangle.h" - +#else +#include #endif Index: lldb/trunk/cmake/modules/LLDBConfig.cmake === --- lldb/trunk/cmake/modules/LLDBConfig.cmake +++ lldb/trunk/cmake/modules/LLDBConfig.cmake @@ -401,3 +401,12 @@ "- ignore this warning and accept occasional instability") endif() endif() + +if(MSVC) +set(LLDB_USE_BUILTIN_DEMANGLER ON) +else() +option(LLDB_USE_BUILTIN_DEMANGLER "Use lldb's builtin demangler instead of the system one" ON) +endif() +if(LLDB_USE_BUILTIN_DEMANGLER) +add_definitions(-DLLDB_USE_BUILTIN_DEMANGLER) +endif() ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r279812 - Switch linux and android CODE_OWNERS
Author: labath Date: Fri Aug 26 05:06:13 2016 New Revision: 279812 URL: http://llvm.org/viewvc/llvm-project?rev=279812&view=rev Log: Switch linux and android CODE_OWNERS Summary: Oleksiy is no longer active in LLDB, I'd like to formally assume ownership of the linux and android parts. Reviewers: ovyalov, clayborg Subscribers: tberghammer, danalbert, lldb-commits Differential Revision: https://reviews.llvm.org/D23877 Modified: lldb/trunk/CODE_OWNERS.txt Modified: lldb/trunk/CODE_OWNERS.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/CODE_OWNERS.txt?rev=279812&r1=279811&r2=279812&view=diff == --- lldb/trunk/CODE_OWNERS.txt (original) +++ lldb/trunk/CODE_OWNERS.txt Fri Aug 26 05:06:13 2016 @@ -49,8 +49,8 @@ N: Zachary Turner E: ztur...@google.com D: CMake build, Host (common+windows), Plugins/Process/Windows, Anything Windows-specific -N: Oleksiy Vyalov -E: ovya...@google.com +N: Pavel Labath +E: lab...@google.com D: Linux, Android N: Todd Fiala ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23877: Switch linux and android CODE_OWNERS
This revision was automatically updated to reflect the committed changes. Closed by commit rL279812: Switch linux and android CODE_OWNERS (authored by labath). Changed prior to commit: https://reviews.llvm.org/D23877?vs=69249&id=69335#toc Repository: rL LLVM https://reviews.llvm.org/D23877 Files: lldb/trunk/CODE_OWNERS.txt Index: lldb/trunk/CODE_OWNERS.txt === --- lldb/trunk/CODE_OWNERS.txt +++ lldb/trunk/CODE_OWNERS.txt @@ -49,8 +49,8 @@ E: ztur...@google.com D: CMake build, Host (common+windows), Plugins/Process/Windows, Anything Windows-specific -N: Oleksiy Vyalov -E: ovya...@google.com +N: Pavel Labath +E: lab...@google.com D: Linux, Android N: Todd Fiala Index: lldb/trunk/CODE_OWNERS.txt === --- lldb/trunk/CODE_OWNERS.txt +++ lldb/trunk/CODE_OWNERS.txt @@ -49,8 +49,8 @@ E: ztur...@google.com D: CMake build, Host (common+windows), Plugins/Process/Windows, Anything Windows-specific -N: Oleksiy Vyalov -E: ovya...@google.com +N: Pavel Labath +E: lab...@google.com D: Linux, Android N: Todd Fiala ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23545: Minidump parsing
labath added a comment. Looks fine to me. Adrian, Zachary, any more thoughts here? Comment at: source/Plugins/Process/minidump/MinidumpParser.h:42 @@ +41,3 @@ +public: +explicit MinidumpParser(const lldb::DataBufferSP &data_buf_sp, const MinidumpHeader *header, +const llvm::DenseMap &directory_map); If you have a `Create` function, the constructor should probably be private (also, `explicit` is not necessary anymore). https://reviews.llvm.org/D23545 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23545: Minidump parsing
dvlahovski updated this revision to Diff 69340.
dvlahovski added a comment.
Making MinidumpParser constuctor private
https://reviews.llvm.org/D23545
Files:
cmake/LLDBDependencies.cmake
source/Plugins/Process/CMakeLists.txt
source/Plugins/Process/minidump/CMakeLists.txt
source/Plugins/Process/minidump/MinidumpParser.cpp
source/Plugins/Process/minidump/MinidumpParser.h
source/Plugins/Process/minidump/MinidumpTypes.cpp
source/Plugins/Process/minidump/MinidumpTypes.h
unittests/Process/CMakeLists.txt
unittests/Process/minidump/CMakeLists.txt
unittests/Process/minidump/Inputs/linux-x86_64.dmp
unittests/Process/minidump/MinidumpParserTest.cpp
Index: unittests/Process/minidump/MinidumpParserTest.cpp
===
--- /dev/null
+++ unittests/Process/minidump/MinidumpParserTest.cpp
@@ -0,0 +1,97 @@
+//===-- MinidumpTypesTest.cpp ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+// Project includes
+#include "Plugins/Process/minidump/MinidumpParser.h"
+#include "Plugins/Process/minidump/MinidumpTypes.h"
+
+// Other libraries and framework includes
+#include "gtest/gtest.h"
+
+#include "lldb/Core/ArchSpec.h"
+#include "lldb/Core/DataExtractor.h"
+#include "lldb/Host/FileSpec.h"
+
+#include "llvm/ADT/Optional.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+
+// C includes
+
+// C++ includes
+#include
+
+extern const char *TestMainArgv0;
+
+using namespace lldb_private;
+using namespace minidump;
+
+class MinidumpParserTest : public testing::Test
+{
+public:
+void
+SetUp() override
+{
+llvm::StringRef dmp_folder = llvm::sys::path::parent_path(TestMainArgv0);
+inputs_folder = dmp_folder;
+llvm::sys::path::append(inputs_folder, "Inputs");
+}
+
+void
+SetUpData(const char *minidump_filename, size_t load_size = SIZE_MAX)
+{
+llvm::SmallString<128> filename = inputs_folder;
+llvm::sys::path::append(filename, minidump_filename);
+FileSpec minidump_file(filename.c_str(), false);
+lldb::DataBufferSP data_sp(minidump_file.MemoryMapFileContents(0, load_size));
+llvm::Optional optional_parser = MinidumpParser::Create(data_sp);
+ASSERT_TRUE(optional_parser.hasValue());
+parser.reset(new MinidumpParser(optional_parser.getValue()));
+ASSERT_GT(parser->GetByteSize(), 0UL);
+}
+
+llvm::SmallString<128> inputs_folder;
+std::unique_ptr parser;
+};
+
+TEST_F(MinidumpParserTest, GetThreads)
+{
+SetUpData("linux-x86_64.dmp");
+llvm::Optional> thread_list;
+
+thread_list = parser->GetThreads();
+ASSERT_TRUE(thread_list.hasValue());
+ASSERT_EQ(1UL, thread_list->size());
+
+const MinidumpThread *thread = thread_list.getValue()[0];
+ASSERT_EQ(16001UL, thread->thread_id);
+}
+
+TEST_F(MinidumpParserTest, GetThreadsTruncatedFile)
+{
+SetUpData("linux-x86_64.dmp", 200);
+llvm::Optional> thread_list;
+
+thread_list = parser->GetThreads();
+ASSERT_FALSE(thread_list.hasValue());
+}
+
+TEST_F(MinidumpParserTest, GetArchitecture)
+{
+SetUpData("linux-x86_64.dmp");
+ASSERT_EQ(llvm::Triple::ArchType::x86_64, parser->GetArchitecture().GetTriple().getArch());
+}
+
+TEST_F(MinidumpParserTest, GetMiscInfo)
+{
+SetUpData("linux-x86_64.dmp");
+const MinidumpMiscInfo *misc_info = parser->GetMiscInfo();
+ASSERT_EQ(nullptr, misc_info);
+// linux breakpad generated minidump files don't have misc info stream
+}
Index: unittests/Process/minidump/CMakeLists.txt
===
--- /dev/null
+++ unittests/Process/minidump/CMakeLists.txt
@@ -0,0 +1,8 @@
+add_lldb_unittest(LLDBMinidumpTests
+ MinidumpParserTest.cpp
+ )
+
+set(test_inputs
+ linux-x86_64.dmp)
+
+add_unittest_inputs(LLDBMinidumpTests "${test_inputs}")
Index: unittests/Process/CMakeLists.txt
===
--- unittests/Process/CMakeLists.txt
+++ unittests/Process/CMakeLists.txt
@@ -1 +1,2 @@
add_subdirectory(gdb-remote)
+add_subdirectory(minidump)
Index: source/Plugins/Process/minidump/MinidumpTypes.h
===
--- /dev/null
+++ source/Plugins/Process/minidump/MinidumpTypes.h
@@ -0,0 +1,298 @@
+//===-- MinidumpTypes.h -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef liblldb_MinidumpTypes_h_
+#defi
Re: [Lldb-commits] [PATCH] D23545: Minidump parsing
zturner accepted this revision.
zturner added a reviewer: zturner.
This revision is now accepted and ready to land.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:60
@@ +59,3 @@
+MinidumpParser parser(data_buf_sp, header, directory_map);
+return llvm::Optional(parser);
+}
You can just write `return parser` here. It's implicitly convertible to an
`llvm::Optional<>`.
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:65
@@ +64,3 @@
+ const llvm::DenseMap &directory_map)
+: m_data_sp(data_buf_sp), m_header(header), m_directory_map(directory_map)
+{
This isn't really a performance critical class, so it doesn't matter too much,
but you could `std::move(directory_map)` to avoid the copy.
https://reviews.llvm.org/D23545
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
Unless we have a good reason to use const char* such as interoperability
with a C api I would strongly prefer we standardize on llvm::StringRef
On Fri, Aug 26, 2016 at 2:24 AM Pavel Labath via lldb-commits <
lldb-commits@lists.llvm.org> wrote:
> labath added a comment.
>
> Thank you for writing the tests. I have two stylistic comments, but
> otherwise looks great.
>
>
>
> Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:113
> @@ -105,4 +112,3 @@
> case 'J':
> -// Asynchronous JSON packet, destined for a
> -// StructuredDataPlugin.
> {
> +// Verify this J packet is a JSON-async: packet.
>
> I'd like to move this code to a separate function. The main job of
> `SendContinuePacketAndWaitForResponse` is dealing with the thread
> synchronization issues, which is tricky enough without having json parsing
> in the middle of it.
>
>
> Comment at: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp:371
> @@ +370,3 @@
> +ASSERT_EQ("T01", response.GetStringRef());
> +ASSERT_EQ(fix.delegate.structured_data_entries.size(), 1);
> +
>
> Please put the "expected" values first (`ASSERT_NE(expected, actual)`).
> Otherwise the error message will come out wrong when the comparison fails.
> The same issue is present in a number of other checks.
>
>
> https://reviews.llvm.org/D23884
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
clayborg added a comment.
Looking closer at the JSON parser llvm::StringRef isn't the right thing to use
when parsing JSON. The parser will often remove desensitizing characters from
say a string like:
"hello \"world\""
And when parsing a token in JSONParser::GetToken, we can't just hand out a
llvm::StringRef do this data in memory. The function prototype is:
JSONParser::Token
JSONParser::GetToken (std::string &value)
This would mean we would need "value" to be able to have a new backing store in
case the values don't match what is in the JSON text buffer which would defeat
the purpose of using llvm::StringRef. We could make a struct like:
struct ParseString
{
llvm::StringRef value;
std::string backing_store;
};
But then the code becomes more complex for no real benefit.
https://reviews.llvm.org/D23884
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
zturner added a subscriber: zturner. zturner added a comment. I'm not sure I follow. `StringRef` is literally just a wrapper around a `const char*`, so if `const char*` works, why doesn't `StringRef`? `JSONParser` constructor is already taking a `const char*` in this patch, so it's not like it is modifying the contents of the string. Checking the length and/or getting a substring or trimming values from the beginning and end is going to be a far more common operation than converting it into something that has to be null terminated (why does it have to be null terminated for that matter anyway?), so the benefit of having the length stored with the string outweighs the con of having to call `str()` when you want something null terminated, IMO https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
zturner added a comment. Actually it looks like `JSONParser` constructor takes a `StringRef` and then converts it to a `std::string` internally. Why can't it just store the `StringRef` internally? It doesn't modify the buffer, and incidentally it also keeps a `uint64_t` representing the "offset" into the string. That's what a `StringRef` is. an immutable buffer and an offset. Each time you extract something, you just say `m_buffer = m_buffer.drop_front(n)`. There is the `SetFilePos()` method, but it looks like it's only being used for two purposes: 1) To move forward, and 2) To reset the position to 0 after setting a new string in the extractor. The first one is still possible with a `StringRef` by writing `extractor.skip(n)`, and the second one is still possible by writing `extractor.SetString(NewStringRef)`. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. We should probably move over to using llvm::StrringRef instead of "const char *". It the JSON parser isn't using llvm::StringRef already it probably should. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
tfiala added a comment. Greg's going to change over the parser now to StringRef. I'll adjust my patch once he does that. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Actually after looking at the JSON parser I see that it isn't using llvm::StringRef throughout. So I vote to keep the "const char *" for now since we need to guarantee that the JSON is null terminated. If we take a llvm::StringRef, then we will need to call llvm::StringRef::str() to make a std::string so we can guarantee that the JSON is null terminated which we do not want to have to do. Either we include changing the JSON parser over to using llvm::StringRef throughout and change the constructors, or we leave things as "const char *" for now. I am assuming Todd doesn't want to take on the conversion over to llvm::StringRef so I am going to say I am ok with this change. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
clayborg added a comment. In https://reviews.llvm.org/D23884#526385, @zturner wrote: > I'm not sure I follow. `StringRef` is literally just a wrapper around a > `const char*`, so if `const char*` works, why doesn't `StringRef`? "const char *" assumed null termination. StringRef can be a reference to part of a C string: const char *cstr = "\"valid json string\"and some other junk"; llvm::StringRef json_text(cstr, 19); Not that the last character after the ending quote isn't a '\0' charcter it is a 'a'. So if we want to actually use the "json_text" to parse the JSON, we need make a std::string so that it becomes null terminated. We could switch the parsing code to use a new class StringRefExtractor instead of StringExtractor in which case we can accept the data as a llvm::StringRef from the constructor. But all internal code would need to use std::string (GetToken() would still need to use std::string). > `JSONParser` constructor is already taking a `const char*` in this patch, so > it's not like it is modifying the contents of the string. You are correct. It does make a contract that used to not be there where the caller must guarantee the data it is parsing doesn't go away, but I don't think that will be a problem. > Checking the length and/or getting a substring or trimming values from the > beginning and end is going to be a far more common operation than converting > it into something that has to be null terminated (why does it have to be null > terminated for that matter anyway?) Currently it is so the parser knows when to stop, but it doesn't have to be if we switch to using a new StringRefExtractor class. > So the benefit of having the length stored with the string outweighs the con > of having to call `str()` when you want something null terminated, IMO Yes it it fine as long as you convert the parsing code. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
> Greg's going to change over the parser now to StringRef. I'll adjust my patch once he does that. Sorry, that comment was incorrect and I removed from the website about 30-ish minutes ago. I'm not sure why it just popped up now. Disregard. On Fri, Aug 26, 2016 at 8:54 AM, Todd Fiala wrote: > tfiala added a comment. > > Greg's going to change over the parser now to StringRef. I'll adjust my > patch once he does that. > > > https://reviews.llvm.org/D23884 > > > > -- -Todd ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
clayborg added a comment. In https://reviews.llvm.org/D23884#526390, @zturner wrote: > Actually it looks like `JSONParser` constructor takes a `StringRef` and then > converts it to a `std::string` internally. Why can't it just store the > `StringRef` internally? It doesn't modify the buffer, and incidentally it > also keeps a `uint64_t` representing the "offset" into the string. That's > what a `StringRef` is. an immutable buffer and an offset. Each time you > extract something, you just say `m_buffer = m_buffer.drop_front(n)`. Yep, we just need to create a StringRefExtractor class that uses a llvm::StringRef class internally instead of a std::string. If you do something about this, don't change StringExtractor since all of the GDB remote packets use these and actually use the std::string as the backing store for all of its incoming packets. > There is the `SetFilePos()` method, but it looks like it's only being used > for two purposes: 1) To move forward, and 2) To reset the position to 0 > after setting a new string in the extractor. The first one is still possible > with a `StringRef` by writing `extractor.skip(n)`, and the second one is > still possible by writing `extractor.SetString(NewStringRef)`. You don't want to modify the string in any way. StringRefExtractor would contain a llvm::StringRef + the current offset position just like StringExtractor. No need to change anything functionality wise. Again, if we switch the parser over to use an llvm::StringRef then this make sense, otherwise it doesn't. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
zturner added a comment. You mention a `StringRefExtractor`. But nothing about `StringExtractor` that I can tell depends on the fact that it's null terminated. It only needs to know where to stop. A `StringRef` contains a length, so you stop when it gets to the end of the `StringRef`. It seems to me like we don't need a `StringRefExtractor`, but rather that `StringExtractor` itself can just be updated to use a `StringRef` instead of a `std::string`. Anyway, I will try to whip up a patch that converts `StringExtractor` to store a `StringRef` internally. I'll put it up for review when I'm done just so you can see what I have in mind. If it works great, if it doesn't oh well :) https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
tfiala added a comment. > Again, if we switch the parser over to use an llvm::StringRef then this make > sense, otherwise it doesn't. I don't want to conflate this change with switching the parser over to a string ref. The only reason I changed the entry to a const char* was to avoid creating a couple unnecessary temporaries. We can consider the parser conversion a separate logical change that can be done independently. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
tfiala added inline comments.
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:113
@@ -105,4 +112,3 @@
case 'J':
-// Asynchronous JSON packet, destined for a
-// StructuredDataPlugin.
{
+// Verify this J packet is a JSON-async: packet.
labath wrote:
> I'd like to move this code to a separate function. The main job of
> `SendContinuePacketAndWaitForResponse` is dealing with the thread
> synchronization issues, which is tricky enough without having json parsing in
> the middle of it.
Sure, sounds like a good change.
Comment at: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp:371
@@ +370,3 @@
+ASSERT_EQ("T01", response.GetStringRef());
+ASSERT_EQ(fix.delegate.structured_data_entries.size(), 1);
+
labath wrote:
> Please put the "expected" values first (`ASSERT_NE(expected, actual)`).
> Otherwise the error message will come out wrong when the comparison fails.
> The same issue is present in a number of other checks.
Okay. That's somewhat unfortunate that Python unittest's messages are geared
for the reverse:
http://bugs.python.org/issue10573
I will reverse these, thanks for catching that!
https://reviews.llvm.org/D23884
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23545: Minidump parsing
amccarth added a comment. LGTM. https://reviews.llvm.org/D23545 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D23884: Add StructuredData unit tests; remove JSON parsing string copy
tfiala added a comment. In https://reviews.llvm.org/D23884#526419, @tfiala wrote: > > Again, if we switch the parser over to use an llvm::StringRef then this > > make sense, otherwise it doesn't. > > > I don't want to conflate this change with switching the parser over to a > string ref. The only reason I changed the entry to a const char* was to > avoid creating a couple unnecessary temporaries. > > We can consider the parser conversion a separate logical change that can be > done independently. Greg and I talked about this a bit. I'm going to reverse out the change to the JSON parsing signature, and just add a move-enabled one to avoid the temporary I was concerned about. https://reviews.llvm.org/D23884 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r279850 - Don't crash when trying to capture persistent variables in a block.
Author: spyffe
Date: Fri Aug 26 13:12:39 2016
New Revision: 279850
URL: http://llvm.org/viewvc/llvm-project?rev=279850&view=rev
Log:
Don't crash when trying to capture persistent variables in a block.
Reports an error instead. We can fix this later to make persistent variables
work, but right now we hit an LLVM assertion if we get this wrong.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/c/blocks/TestBlocks.py
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.h
Modified: lldb/trunk/packages/Python/lldbsuite/test/lang/c/blocks/TestBlocks.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/c/blocks/TestBlocks.py?rev=279850&r1=279849&r2=279850&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/blocks/TestBlocks.py
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/blocks/TestBlocks.py Fri
Aug 26 13:12:39 2016
@@ -57,8 +57,10 @@ class BlocksTestCase(TestBase):
self.launch_common()
self.runCmd("expression int (^$add)(int, int) = ^int(int a, int b) {
return a + b; };")
-
self.expect("expression $add(2,3)", VARIABLES_DISPLAYED_CORRECTLY,
substrs = [" = 5"])
+
+self.runCmd("expression int $a = 3")
+self.expect("expression int (^$addA)(int) = ^int(int b) { return $a +
b; };", "Proper error is reported on capture", error=True)
def wait_for_breakpoint(self):
if self.is_started == False:
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=279850&r1=279849&r2=279850&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Fri Aug 26 13:12:39 2016
@@ -885,7 +885,7 @@ ClangExpressionParser::PrepareForExecuti
Process *process = exe_ctx.GetProcessPtr();
-if (execution_policy != eExecutionPolicyAlways && execution_policy !=
eExecutionPolicyTopLevel)
+if (execution_policy != eExecutionPolicyAlways && execution_policy !=
eExecutionPolicyTopLevel && ir_can_run)
{
lldb_private::Error interpret_error;
Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp?rev=279850&r1=279849&r2=279850&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp Fri Aug 26
13:12:39 2016
@@ -552,7 +552,7 @@ IRForTarget::RewriteObjCConstString (llv
llvm::cast(m_entry_instruction_finder.GetValue(function)));
});
-if (!UnfoldConstant(ns_str, CFSCWB_Caller, m_entry_instruction_finder))
+if (!UnfoldConstant(ns_str, nullptr, CFSCWB_Caller,
m_entry_instruction_finder, nullptr))
{
if (log)
log->PutCString("Couldn't replace the NSString with the result of
the call");
@@ -1602,8 +1602,10 @@ IRForTarget::RemoveGuards(BasicBlock &ba
// This function does not report errors; its callers are responsible.
bool
IRForTarget::UnfoldConstant(Constant *old_constant,
+llvm::Function *llvm_function,
FunctionValueCache &value_maker,
-FunctionValueCache &entry_instruction_finder)
+FunctionValueCache &entry_instruction_finder,
+lldb_private::Stream *error_stream)
{
lldb_private::Log *log(lldb_private::GetLogIfAllCategoriesSet
(LIBLLDB_LOG_EXPRESSIONS));
@@ -1647,7 +1649,7 @@ IRForTarget::UnfoldConstant(Constant *ol
llvm::cast(entry_instruction_finder.GetValue(function)));
});
-if (!UnfoldConstant(constant_expr, bit_cast_maker,
entry_instruction_finder))
+if (!UnfoldConstant(constant_expr, llvm_function,
bit_cast_maker, entry_instruction_finder, error_stream))
return false;
}
break;
@@ -1685,7 +1687,7 @@ IRForTarget::UnfoldConstant(Constant *ol
return GetElementPtrInst::Create(nullptr, ptr,
indices, "",
llvm::cast(entry_instruction_finder.GetValue(function)));
});
-if (!U
[Lldb-commits] LLVM buildmaster will be restarted tonight
Hello everyone, LLVM buildmaster will be restarted after 6 PM Pacific time today. Thanks Galina ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D22286: [LLDB] Help text overhaul
k8stone accepted this revision. k8stone added a reviewer: k8stone. k8stone added a comment. All changes long since accepted and submitted to trunk. https://reviews.llvm.org/D22286 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r279884 - Fixed a bad lldbassert() condition.
Author: spyffe
Date: Fri Aug 26 18:48:03 2016
New Revision: 279884
URL: http://llvm.org/viewvc/llvm-project?rev=279884&view=rev
Log:
Fixed a bad lldbassert() condition.
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=279884&r1=279883&r2=279884&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Fri Aug 26 18:48:03 2016
@@ -277,7 +277,7 @@ ClangExpressionParser::ClangExpressionPa
target_sp = exe_scope->CalculateTarget();
if (!target_sp)
{
-lldb_assert(exe_scope, "Can't make an expression parser with a null
target.", __FUNCTION__, __FILE__, __LINE__);
+lldb_assert(target_sp.get(), "Can't make an expression parser with a
null target.", __FUNCTION__, __FILE__, __LINE__);
return;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r279894 - The error stream in IRForTarget is never null, so use it instead of the log.
Author: spyffe
Date: Fri Aug 26 19:20:38 2016
New Revision: 279894
URL: http://llvm.org/viewvc/llvm-project?rev=279894&view=rev
Log:
The error stream in IRForTarget is never null, so use it instead of the log.
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.h
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=279894&r1=279893&r2=279894&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Fri Aug 26 19:20:38 2016
@@ -875,10 +875,9 @@ ClangExpressionParser::PrepareForExecuti
{
Stream *error_stream = NULL;
Target *target = exe_ctx.GetTargetPtr();
-if (target)
-error_stream = target->GetDebugger().GetErrorFile().get();
+error_stream = target->GetDebugger().GetErrorFile().get();
-IRForTarget ir_for_target(decl_map, m_expr.NeedsVariableResolution(),
*execution_unit_sp, error_stream,
+IRForTarget ir_for_target(decl_map, m_expr.NeedsVariableResolution(),
*execution_unit_sp, *error_stream,
function_name.AsCString());
bool ir_can_run =
ir_for_target.runOnModule(*execution_unit_sp->GetModule());
Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp?rev=279894&r1=279893&r2=279894&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp (original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp Fri Aug 26
19:20:38 2016
@@ -78,7 +78,7 @@ FindEntryInstruction (llvm::Function *fu
IRForTarget::IRForTarget (lldb_private::ClangExpressionDeclMap *decl_map,
bool resolve_vars,
lldb_private::IRExecutionUnit &execution_unit,
- lldb_private::Stream *error_stream,
+ lldb_private::Stream &error_stream,
const char *func_name) :
ModulePass(ID),
m_resolve_vars(resolve_vars),
@@ -239,8 +239,7 @@ IRForTarget::CreateResultVariable (llvm:
if (log)
log->PutCString("Result variable had no data");
-if (m_error_stream)
-m_error_stream->Printf("Internal error [IRForTarget]: Result
variable's name (%s) exists, but not its definition\n", result_name);
+m_error_stream.Printf("Internal error [IRForTarget]: Result variable's
name (%s) exists, but not its definition\n", result_name);
return false;
}
@@ -255,8 +254,7 @@ IRForTarget::CreateResultVariable (llvm:
if (log)
log->PutCString("Result variable isn't a GlobalVariable");
-if (m_error_stream)
-m_error_stream->Printf("Internal error [IRForTarget]: Result
variable (%s) is defined, but is not a global variable\n", result_name);
+m_error_stream.Printf("Internal error [IRForTarget]: Result variable
(%s) is defined, but is not a global variable\n", result_name);
return false;
}
@@ -267,8 +265,7 @@ IRForTarget::CreateResultVariable (llvm:
if (log)
log->PutCString("Result variable doesn't have a corresponding
Decl");
-if (m_error_stream)
-m_error_stream->Printf("Internal error [IRForTarget]: Result
variable (%s) does not have a corresponding Clang entity\n", result_name);
+m_error_stream.Printf("Internal error [IRForTarget]: Result variable
(%s) does not have a corresponding Clang entity\n", result_name);
return false;
}
@@ -289,8 +286,7 @@ IRForTarget::CreateResultVariable (llvm:
if (log)
log->PutCString("Result variable Decl isn't a VarDecl");
-if (m_error_stream)
-m_error_stream->Printf("Internal error [IRForTarget]: Result
variable (%s)'s corresponding Clang entity isn't a variable\n", result_name);
+m_error_stream.Printf("Internal error [IRForTarget]: Result variable
(%s)'s corresponding Clang entity isn't a variable\n", result_name);
return false;
}
@@ -327,8 +323,7 @@ IRForTarget::CreateResultVariable (llvm:
if (log)
log->PutCString("Expected result to have pointer type, but it
did not");
-if (m_error_stream)
-m_error_stream->Printf("Internal error [IRForTarget]: Lvalue
result (%s) is not a pointer variable\n", result_name);
+
[Lldb-commits] [lldb] r279896 - Fixed the location of a conditional to make the following code clearer.
Author: spyffe
Date: Fri Aug 26 19:35:37 2016
New Revision: 279896
URL: http://llvm.org/viewvc/llvm-project?rev=279896&view=rev
Log:
Fixed the location of a conditional to make the following code clearer.
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp?rev=279896&r1=279895&r2=279896&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
Fri Aug 26 19:35:37 2016
@@ -882,9 +882,15 @@ ClangExpressionParser::PrepareForExecuti
bool ir_can_run =
ir_for_target.runOnModule(*execution_unit_sp->GetModule());
+if (!ir_can_run)
+{
+err.SetErrorString("The expression could not be prepared to run in
the target");
+return err;
+}
+
Process *process = exe_ctx.GetProcessPtr();
-if (execution_policy != eExecutionPolicyAlways && execution_policy !=
eExecutionPolicyTopLevel && ir_can_run)
+if (execution_policy != eExecutionPolicyAlways && execution_policy !=
eExecutionPolicyTopLevel)
{
lldb_private::Error interpret_error;
@@ -900,12 +906,6 @@ ClangExpressionParser::PrepareForExecuti
}
}
-if (!ir_can_run)
-{
-err.SetErrorString("The expression could not be prepared to run in
the target");
-return err;
-}
-
if (!process && execution_policy == eExecutionPolicyAlways)
{
err.SetErrorString("Expression needed to run in the target, but
the target can't be run");
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r279897 - This test now succeeds.
Author: jingham
Date: Fri Aug 26 19:35:48 2016
New Revision: 279897
URL: http://llvm.org/viewvc/llvm-project?rev=279897&view=rev
Log:
This test now succeeds.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py?rev=279897&r1=279896&r2=279897&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py Fri
Aug 26 19:35:48 2016
@@ -17,7 +17,6 @@ class TestQueues(TestBase):
@skipUnlessDarwin
@add_test_categories(['pyapi'])
-@unittest2.expectedFailure("rdar://22531180")
def test_with_python_api(self):
"""Test queues inspection SB APIs."""
self.build()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D23948: Remove unused variable
wttsugrii created this revision.
wttsugrii added a reviewer: hhellyer.
wttsugrii added a subscriber: lldb-commits.
range_base variable is not used anywhere, so can be
safely removed.
https://reviews.llvm.org/D23948
Files:
source/Target/Process.cpp
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -6603,7 +6603,6 @@
Error error;
-lldb::addr_t range_base = 0;
lldb::addr_t range_end = 0;
region_list.clear();
@@ -6618,7 +6617,6 @@
break;
}
-range_base = region_info->GetRange().GetRangeBase();
range_end = region_info->GetRange().GetRangeEnd();
if( region_info->GetMapped() == MemoryRegionInfo::eYes )
{
Index: source/Target/Process.cpp
===
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -6603,7 +6603,6 @@
Error error;
-lldb::addr_t range_base = 0;
lldb::addr_t range_end = 0;
region_list.clear();
@@ -6618,7 +6617,6 @@
break;
}
-range_base = region_info->GetRange().GetRangeBase();
range_end = region_info->GetRange().GetRangeEnd();
if( region_info->GetMapped() == MemoryRegionInfo::eYes )
{
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D23950: Remove unused local variable
wttsugrii created this revision.
wttsugrii added a reviewer: granata.enrico.
wttsugrii added a subscriber: lldb-commits.
type variable is not used and can be safely removed.
https://reviews.llvm.org/D23950
Files:
source/DataFormatters/TypeCategoryMap.cpp
Index: source/DataFormatters/TypeCategoryMap.cpp
===
--- source/DataFormatters/TypeCategoryMap.cpp
+++ source/DataFormatters/TypeCategoryMap.cpp
@@ -394,7 +394,6 @@
{
if (pos->second->IsEnabled())
continue;
-KeyType type = pos->first;
if (!callback(pos->second))
break;
}
Index: source/DataFormatters/TypeCategoryMap.cpp
===
--- source/DataFormatters/TypeCategoryMap.cpp
+++ source/DataFormatters/TypeCategoryMap.cpp
@@ -394,7 +394,6 @@
{
if (pos->second->IsEnabled())
continue;
-KeyType type = pos->first;
if (!callback(pos->second))
break;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D23951: Remove unused any_found local variable
wttsugrii created this revision. wttsugrii added a reviewer: granata.enrico. wttsugrii added a subscriber: lldb-commits. any_found variable is not used anywhere and can be safely removed. https://reviews.llvm.org/D23951 Files: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp === --- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp +++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp @@ -1751,7 +1751,6 @@ bool success = false; -bool any_found = false; diagnostics.Clear(); @@ -1812,7 +1811,7 @@ process->GetByteOrder(), addr_size); -any_found = (ParseClassInfoArray (class_infos_data, num_class_infos) > 0); +ParseClassInfoArray (class_infos_data, num_class_infos); } } else Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp === --- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp +++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp @@ -1751,7 +1751,6 @@ bool success = false; -bool any_found = false; diagnostics.Clear(); @@ -1812,7 +1811,7 @@ process->GetByteOrder(), addr_size); -any_found = (ParseClassInfoArray (class_infos_data, num_class_infos) > 0); +ParseClassInfoArray (class_infos_data, num_class_infos); } } else ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D23952: Remove unused frame_zero_id local variable
wttsugrii created this revision. wttsugrii added a reviewer: jingham. wttsugrii added a subscriber: lldb-commits. frame_zero_id local variable is not used and can be safely removed. https://reviews.llvm.org/D23952 Files: source/Target/ThreadPlanStepOut.cpp Index: source/Target/ThreadPlanStepOut.cpp === --- source/Target/ThreadPlanStepOut.cpp +++ source/Target/ThreadPlanStepOut.cpp @@ -71,8 +71,6 @@ m_step_out_to_id = return_frame_sp->GetStackID(); m_immediate_step_from_id = immediate_return_from_sp->GetStackID(); - -StackID frame_zero_id = m_thread.GetStackFrameAtIndex(0)->GetStackID(); // If the frame directly below the one we are returning to is inlined, we have to be // a little more careful. It is non-trivial to determine the real "return code address" for Index: source/Target/ThreadPlanStepOut.cpp === --- source/Target/ThreadPlanStepOut.cpp +++ source/Target/ThreadPlanStepOut.cpp @@ -71,8 +71,6 @@ m_step_out_to_id = return_frame_sp->GetStackID(); m_immediate_step_from_id = immediate_return_from_sp->GetStackID(); - -StackID frame_zero_id = m_thread.GetStackFrameAtIndex(0)->GetStackID(); // If the frame directly below the one we are returning to is inlined, we have to be // a little more careful. It is non-trivial to determine the real "return code address" for ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
