[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

labath added inline comments. Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:305 if (is_trivially_serializable::value) - return; + return const_cast(t); // We need to make a copy as the original object might go out of scope.

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

kwk added a comment. In D75750#1971446 , @labath wrote: > In D75750#1967019 , @fche2 wrote: > > > >> So it might be good to have the SymbolVendors use one or more > > >> SymbolServer plug-ins. > > > > > > I don't

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

labath added a comment. While getting rid of this would be great, this (the manual dwarf index) is also very performance sensitive code which has been optimized and re-optimized a number of times. So, I would want to see a benchmark to ensure this does not affect performance adversely. A good b

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

labath added a comment. lldb-dev is indeed a better place for the architectural discussion. However, moving the discussion there does not automatically unblock this patch. "get something in now and improve the architecture later" almost never works out in practice. In fact I would say that addi

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

kwk resigned from this revision. kwk added a comment. This revision now requires review to proceed. I resign because I think @labath made some good points that I cannot argue about. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78337/new/ https://reviews.llvm.

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

kwk accepted this revision. kwk added a comment. This revision is now accepted and ready to land. All tests pass. I first thought that the `lldb/test/Shell/Reproducer/TestGDBRemoteRepro.test` test didn't work but it seems to be flaky. Comment at: lldb/source/Plugins/SymbolFi

[Lldb-commits] [PATCH] D78337: [lldb/Host] Remove TaskPool and replace its uses with llvm::ThreadPool

labath added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:24-30 +static void TaskMapOverInt(size_t end, + const llvm::function_ref &func) { + llvm::ThreadPool pool; + for (size_t i = 0; i < end; i++) +pool.

[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix passive replay for functions returning string as (char*, size_t).

labath added a comment. Hmm... well, I like how this (active&passive) is unified at the template level. It still feels like there's too much code to implement this thing (coming from the duplication for const, non-const and static variants), but I'm not sure what can be done about that. I'll ne

Re: [Lldb-commits] [lldb] 1fae85a - [DWARF] Add instructions to regenerate this test, if needed.

Hi Davide, I reread the review, and I see I was confused by two things: (1) the name of the test is static_scope.s even though what it's testing is the scope of a var with a const_value, nothing to do with `static`; (2) there's this comment in the review: https://reviews.llvm.org/D77698#1973122

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

kwk added a subscriber: fche. kwk added a comment. In D75750#1988330 , @labath wrote: > lldb-dev is indeed a better place for the architectural discussion. However, > moving the discussion there does not automatically unblock this patch. "get > something

[Lldb-commits] [PATCH] D75750: [lldb] integrate debuginfod

jankratochvil added a comment. The current plan discussed with @kwk is to create the new `SymbolServer` abstract superclass and some its inherited implementation and move there the appropriate parts of existing `lldb/source/Symbol/LocateSymbolFile.cpp`. Current `SymbolVendor` implementations wo

[Lldb-commits] [PATCH] D78329: Allow lldb-test to combine -find with -dump-clang-ast

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Thank you! This looks like a nice update to the functionality. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78329/new/ https://reviews.llvm.org/D78329 __

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

aprantl added inline comments. Comment at: lldb/unittests/DataFormatter/StringPrinterTests.cpp:1 +//===-- StringPrinterTests.cpp --*- C++ -*-===// +// drop the redundant C++ marker Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [lldb] 681466f - Allow lldb-test to combine -find with -dump-clang-ast

Author: Adrian Prantl Date: 2020-04-17T11:01:20-07:00 New Revision: 681466f5e6412350a0b066791450e72325c2c074 URL: https://github.com/llvm/llvm-project/commit/681466f5e6412350a0b066791450e72325c2c074 DIFF: https://github.com/llvm/llvm-project/commit/681466f5e6412350a0b066791450e72325c2c074.diff

[Lldb-commits] [PATCH] D78329: Allow lldb-test to combine -find with -dump-clang-ast

This revision was automatically updated to reflect the committed changes. Closed by commit rG681466f5e641: Allow lldb-test to combine -find with -dump-clang-ast (authored by aprantl). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[Lldb-commits] [PATCH] D77602: [lldb/Reproducers] Support new replay mode: passive replay

JDevlieghere updated this revision to Diff 258390. JDevlieghere marked an inline comment as done. JDevlieghere added a comment. - Remove const_cast - Initialize instrumentation data in `SBReproducer.cpp` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77602/new/ https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

mib created this revision. mib added reviewers: friss, davide, jingham. mib added a project: LLDB. Herald added subscribers: lldb-commits, mgorny. mib edited the summary of this revision. mib updated this revision to Diff 258402. mib added a comment. Reformat patch. [lldb/Dataformatter] Add supp

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

mib updated this revision to Diff 258402. mib added a comment. Reformat patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78396/new/ https://reviews.llvm.org/D78396 Files: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp lldb/source/Plugin

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

davide added a comment. Thanks for taking care of this. It's a lot of work. First round of comments. Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:12 + +CFBasicHash::~CFBasicHash() { m_address = LLDB_INVALID_ADDRESS; } + Why do you need this? Ca

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

vsk marked 8 inline comments as done. vsk added inline comments. Comment at: lldb/source/DataFormatters/StringPrinter.cpp:173 + unsigned escaped_len; + const unsigned max_buffer_size = 7; + uint8_t *data = new uint8_t[max_buffer_size]; shafik wrote: > `constex

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

vsk marked an inline comment as done. vsk added inline comments. Comment at: lldb/source/DataFormatters/StringPrinter.cpp:405 -template <> -bool StringPrinter::ReadStringAndDumpToStream< -StringPrinter::StringElementType::ASCII>( This is what I mean. The whol

[Lldb-commits] [PATCH] D77843: [lldb/DataFormatters] Delete GetStringPrinterEscapingHelper

vsk updated this revision to Diff 258419. vsk marked an inline comment as done. vsk added a comment. Address review feedback: - Use standard gtest operators (EXPECT_EQ) - constexpr/std::move cleanup, comment cleanups, sprintf length fix Also, I took the opportunity to further unify the ASCII and

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

mib marked 11 inline comments as done. mib added inline comments. Comment at: lldb/source/Plugins/Language/ObjC/CFBasicHash.cpp:35-71 + +size = sizeof(__CFBasicHash::RuntimeBase); +size += sizeof(__CFBasicHash::Bits); + +DataBufferHeap buffer(size, 0); +m_exe_ctx_

[Lldb-commits] [PATCH] D78396: [lldb/Dataformatter] Add support for CoreFoundation Dictionaries and Sets

mib updated this revision to Diff 258431. mib marked 2 inline comments as done. mib added a comment. Address Davide's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78396/new/ https://reviews.llvm.org/D78396 Files: lldb/source/Plugins/L

[Lldb-commits] [lldb] eef9cb1 - [lldb] [testsuite] Fix TestFixIts.py on Linux

Author: Jan Kratochvil Date: 2020-04-18T08:32:12+02:00 New Revision: eef9cb1628898adb5652a32eb95b53c544de0d6f URL: https://github.com/llvm/llvm-project/commit/eef9cb1628898adb5652a32eb95b53c544de0d6f DIFF: https://github.com/llvm/llvm-project/commit/eef9cb1628898adb5652a32eb95b53c544de0d6f.diff