[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

davide added inline comments. Comment at: lldb/source/Commands/CommandObjectStats.cpp:37 + +if (target->GetCollectingStats() == true) { + result.AppendError("stats already enabled"); xiaobai wrote: > nit: You can drop the `== true` Thanks, I'll fix these

[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

davide added a comment. In https://reviews.llvm.org/D45547#1066348, @jingham wrote: > Timers seemed like they would be really useful for collection of data about > operations in lldb, but for most things I think they end up being hard to use > because actual wall-clock time is so variable from

[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

davide updated this revision to Diff 142301. https://reviews.llvm.org/D45547 Files: lldb/include/lldb/Target/Target.h lldb/include/lldb/lldb-private-enumerations.h lldb/packages/Python/lldbsuite/test/functionalities/stats/Makefile lldb/packages/Python/lldbsuite/test/functionalities/stats/

[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first

davide added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c:14-15 +// This line adds a real body to the function, so we can set more than one +// breakpoint in it. +printf("Observable side effect\n");

[Lldb-commits] [PATCH] D45554: Make sure deleting all breakpoints clears their sites first

davide added inline comments. Comment at: packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c:14-15 +// This line adds a real body to the function, so we can set more than one +// breakpoint in it. +printf("Observable side effect\n");

[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

davide added a comment. I'm under the impression that we should either merge `log timers` with `stats` or just remove log timers altogether and start from scratch. From what I hear from Jim, it was really useful for a few people, so maybe a fresh start would be a better way of handling things. T

[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

davide added inline comments. Comment at: lldb/source/Commands/CommandObjectStats.cpp:43-46 +target->RegisterStats(StatisticKind::ExpressionSuccessful); +target->RegisterStats(StatisticKind::ExpressionFailure); +target->RegisterStats(StatisticKind::FrameVarSuccess); +

[Lldb-commits] [PATCH] D45573: Report more precise error message when attach fails

davide added a comment. lgtm https://reviews.llvm.org/D45573 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D45480: Move Args.cpp from Interpreter to Utility

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. No objections from me. https://reviews.llvm.org/D45480 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

davide added a comment. I prefer having it as a top level command rather than part of log. If you think about it LLVM does the same distinction and it worked quite well in practice. We plan to collect more metrics to the command so I'd very much like to have this living as a separate entity.

[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

davide added a comment. In https://reviews.llvm.org/D45547#1065054, @jasonmolenda wrote: > Ah, no you couldn't set up a command alias like that. Still, if the full > name was statistics, you could type 'stat' and it would match. 'stats' > wouldn't, though. > > I do think decoupling the disabl

[Lldb-commits] [PATCH] D45547: [Command] Implement `stats`

davide created this revision. davide added reviewers: jingham, friss, JDevlieghere, aprantl, labath, clayborg. This allows us to collect useful metrics about lldb debugging sessions. I thought that an example would be better than a thousand words: Process 19705 stopped * thread #1, queue = '

[Lldb-commits] [PATCH] D45518: [dotest] Use in-tree dsymutil on Darwin

davide accepted this revision. davide added a comment. LGTM https://reviews.llvm.org/D45518 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D45333: WIP: [LIT] Have lit run the lldb test suite

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. Sorry for getting in here late. This seems to be a great improvement on the state of the art, and given it's only enabled for the CMake build, I see little harm not going forward with it. In p

[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

davide added a comment. Can you add another test or two? It's a little complicated to see what's going on here, but from your description, it makes sense. I'm not particularly worried right now to distinguish between `UNSUPPORTED` and `PASS`. In practice, it doesn't matter (at least for the tran

[Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

davide added a comment. Thanks Greg. This is a very large patch, but it seems mostly churn. I'll try to find the time to review it carefully tomorrow. https://reviews.llvm.org/D45170 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://

[Lldb-commits] [PATCH] D44907: Remove Scalar::Cast

This revision was automatically updated to reflect the committed changes. Closed by commit rL328985: [Core] Grab-bag of improvements for Scalar. (authored by davide, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D44907?vs=139844&id=14

[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF

davide closed this revision. davide added a comment. Lang committed this a while ago (r323163) Repository: rL LLVM https://reviews.llvm.org/D41997 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/lis

[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

davide abandoned this revision. davide added a comment. I think this is obsolete by now. https://reviews.llvm.org/D42656 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

davide accepted this revision. davide added a comment. LGTM. I'll commit for you once Greg reviews it again. Comment at: packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/main.c:1 +__attribute__((section("__codesection"))) +int f(int a) { kbaladu

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

davide added a comment. I think this is almost ready to go in modulo minors. I'll let also @labath comment on it. Thanks for your contribution! Do you need somebody to commit this on your behalf? Comment at: packages/Python/lldbsuite/test/arm/breakpoint-thumb-codesection/Test

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

davide added a comment. `lldb-test` for this purpose will be great. there should be examples in `lit/`. Repository: rL LLVM https://reviews.llvm.org/D44998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

[Lldb-commits] [PATCH] D44907: Remove Scalar::Cast

davide added a comment. OK, I'll commit it for you later today. https://reviews.llvm.org/D44907 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

lldb-commits@lists.llvm.org

davide added a comment. Just remove it I'd say (bonus point if you can remove other bits) https://reviews.llvm.org/D44752 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44907: Remove Scalar::Cast

davide added a comment. @tromey thanks! do you need somebody to commit this for you? https://reviews.llvm.org/D44907 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D45011: Prevent double release of mach ports

davide added a comment. I'm not particularly worried about testing double-free behavior, FWIW. Ideally we should, but, I really understand it's a PITA. I think we might get this for free when testing msan/asan (or just running under valgrind), assuming there was already coverage for this path. (

[Lldb-commits] [PATCH] D44998: ObjectFileELF: Add support for arbitrarily named code sections

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. This needs a testcase. Repository: rL LLVM https://reviews.llvm.org/D44998 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

This revision was automatically updated to reflect the committed changes. Closed by commit rL328658: Use the DWARF linkage name when importing C++ methods. (authored by davide, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D40283?vs=1

[Lldb-commits] [PATCH] D44693: Correctly handle float division in Scalar

This revision was automatically updated to reflect the committed changes. Closed by commit rL328649: [Core] Correctly handle float division in Scalar. (authored by davide, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44693 Files: lld

[Lldb-commits] [PATCH] D44693: Correctly handle float division in Scalar

davide added a comment. Allright, we got this one. davide@Davidinos-Mac-Pro ~/w/l/l/lldb> git llvm push Pushing 1 commit: 55f24c19d1c [Core] Correctly handle float division in Scalar. Sendinglldb/trunk/source/Core/Scalar.cpp Sendinglldb/trunk/unittests/Core/ScalarTest.cpp Tran

[Lldb-commits] [PATCH] D44693: Correctly handle float division in Scalar

davide accepted this revision. davide added a comment. LGTM, thanks. Do you have commit access or you need somebody to commit this on your behalf? https://reviews.llvm.org/D44693 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://list

[Lldb-commits] [PATCH] D44693: Correctly handle float division in Scalar

davide added a comment. Yes, this needs a test. Thanks! https://reviews.llvm.org/D44693 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44472: Add and fix some tests for PPC64

davide added a comment. I think I understand why this is failing: AssertionError: False is not True : 'expression ((char**)environ)[0]' returns expected result, got '(char *) $0 = 0x7ffeefbff753 "COMMAND_MODE=unix2003"' Config=x86_64-/Users/davide/work/llvm-monorepo/build-release/bin/clang-7.

[Lldb-commits] [PATCH] D44472: Add and fix some tests for PPC64

davide added a comment. In https://reviews.llvm.org/D44472#1043464, @lbianc wrote: > @davide Are you sure this is the correct patch? This one was not merged yet. > Could you give more details about the issue? Is it related with one of our > changes? Apologies, this is the right one commit 98

[Lldb-commits] [PATCH] D44472: Add and fix some tests for PPC64

davide added a comment. In https://reviews.llvm.org/D44472#1043510, @davide wrote: > In https://reviews.llvm.org/D44472#1043464, @lbianc wrote: > > > @davide Are you sure this is the correct patch? This one was not merged yet. > > Could you give more details about the issue? Is it related with o

[Lldb-commits] [PATCH] D44472: Add and fix some tests for PPC64

davide added subscribers: alexandreyy, labath, davide. davide added a comment. Leonardo, this breaks the Mac OS X bot. I'm going to revert this to get the bots green again, unless you have a super quick fix (please let me know :) https://reviews.llvm.org/D44472 ___

[Lldb-commits] [PATCH] D44502: Fix a bug in "target.source-map" where we would resolve unmapped paths incorrectly

davide accepted this revision. davide added a comment. LGTM, thanks https://reviews.llvm.org/D44502 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44502: Fix a bug in "target.source-map" where we would resolve unmapped paths incorrectly

davide added a comment. (please wait a day or two if @labath has comments) but this should be fine. https://reviews.llvm.org/D44502 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44473: [dotest] Make llvm-dotest a custom target

davide added a comment. The name should be changed (also the utility name), but that should be done separately. Repository: rL LLVM https://reviews.llvm.org/D44473 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/c

[Lldb-commits] [PATCH] D44473: [dotest] Make llvm-dotest a custom target

davide added a comment. Also, I second the feeling of having `lldb` somewhere in the name for the utility (rather than `llvm` :) Repository: rL LLVM https://reviews.llvm.org/D44473 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http:/

[Lldb-commits] [PATCH] D44473: [dotest] Make llvm-dotest a custom target

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D44473 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[Lldb-commits] [PATCH] D42892: DWZ 02/11: Move the codebase to use: DWARFCompileUnit -> DWARFUnit

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D42892 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/ll

[Lldb-commits] [PATCH] D40466: DWZ 01/11: DWARFUnit split out of DWARFCompileUnit

davide accepted this revision. davide added a comment. This is No functional change, right (just code churn)? If so, LGTM. https://reviews.llvm.org/D40466 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[Lldb-commits] [PATCH] D44321: Support demangling for D symbols via dlopen

davide added a comment. In https://reviews.llvm.org/D44321#1034043, @timotheecour wrote: > In https://reviews.llvm.org/D44321#1033325, @davide wrote: > > > This patch has no testcase. It shouldn't be particularly hard to write one, > > you can take inspiration from the one in `lit/`. > > > > Tha

[Lldb-commits] [PATCH] D44321: Support demangling for D symbols via dlopen

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. This patch has no testcase. It shouldn't be particularly hard to write one, you can take inspiration from the one in `lit/`. Thanks! Repository: rL LLVM https://reviews.llvm.org

[Lldb-commits] [PATCH] D44074: ObjectFileMachO: split CreateSections mega-function into more manageable chunks

davide added a comment. Fair, I don't have a strong opinion on whether this should be in an header or not. Probably Greg is right though, if this is not used anywhere else, we could make it somehow private. Thanks! https://reviews.llvm.org/D44074

[Lldb-commits] [PATCH] D44074: ObjectFileMachO: split CreateSections mega-function into more manageable chunks

davide accepted this revision. davide added a comment. LGTM Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h:191-202 + struct SegmentParsingContext { +const EncryptedFileRanges EncryptedRanges; +lldb_private::SectionList &UnifiedList; +uint32_t NextSe

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

davide added a comment. In https://reviews.llvm.org/D32167#1026780, @jankratochvil wrote: > In https://reviews.llvm.org/D32167#1026779, @davide wrote: > > > Do you have a way of reproducing? > > > It just happens for me each time - on Fedora 27 x86_64 on 16-core (32HT) > 2-node NUMA machine havi

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

davide added a comment. In https://reviews.llvm.org/D32167#1026762, @jankratochvil wrote: > In https://reviews.llvm.org/D32167#1024546, @labath wrote: > > > I personally don't think having a new debug info flavour is a good idea. > > Tests written specifically to test this functionality will be

[Lldb-commits] [PATCH] D44058: [lldb] Fix "code unreachable" warning in DNBArchImplX86_64::SetRegisterValue

davide added a comment. This seems scary. We really need a test case for this. https://reviews.llvm.org/D44058 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44055: [lldb] Fix "requires global constructor" warning in g_range_specifiers

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM with Pavel's suggestion implemented. https://reviews.llvm.org/D44055 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. As already pointed out, I think this feature should be thought again & have more focused testing. We can have a meeting/discussion about this, and I need to think about it more. But

[Lldb-commits] [PATCH] D43884: [lldb] Extract more fields from NSException values

davide added a comment. LG modulo the test. Update that, I'll take another look and approve it. Thanks for your contribution! Comment at: packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py:1-4 +# encoding: utf-8 +""" +Test lldb Obj-C exception support.

[Lldb-commits] [PATCH] D43884: [lldb] Extract more fields from NSException values

davide added a comment. I like the way this patch is structured, some comments inline. Comment at: packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py:1-4 +# encoding: utf-8 +""" +Test lldb Obj-C exception support. +""" This looks like it

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

davide added a comment. In https://reviews.llvm.org/D32167#1019621, @labath wrote: > In https://reviews.llvm.org/D32167#1019504, @clayborg wrote: > > > In https://reviews.llvm.org/D32167#1019467, @labath wrote: > > > > > However, I am not so sure about the proliferation of debug info variants >

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. This commit has no tests. It should have many. It's very big, so it could be split in several pieces. I'd really appreciate if you can take the time to do so. For now, some comments.

[Lldb-commits] [PATCH] D40475: DWZ 10/11: DWZ test mode

davide added a comment. fwiw, you don't need unit tests or python tests to implement this. If I understand the feature correctly you can probably extend `lldb-test` (which is run as part of `check-lldb-lit`). Up to you though. https://reviews.llvm.org/D40475 _

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

davide added a comment. (and thanks for saving 1 minutes and 30 seconds of my life multiplied by the many times I run the test suite per day). https://reviews.llvm.org/D43686 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.ll

[Lldb-commits] [PATCH] D43686: Add "lldb-test breakpoint" command and convert the case-sensitivity test to use it

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. I was going to suggest the same thing Zach suggested, but I think this fine as is. LGTM. The fact the test is more concise is definitely a win, but I don't think this is the main reason for do

[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash

davide accepted this revision. davide added a comment. Thanks. https://reviews.llvm.org/D43596 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43591: [testsuite] Run lit tests as part of `check-lldb`

This revision was automatically updated to reflect the committed changes. Closed by commit rL325719: [testsuite] Run lit tests as part of `check-lldb`. (authored by davide, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D43591?vs=13531

[Lldb-commits] [PATCH] D43591: [testsuite] Run lit tests as part of `check-lldb`

davide created this revision. davide added reviewers: friss, vsk, JDevlieghere, labath, zturner. Herald added a subscriber: mgorny. Also, fix a missing dependency, as lit requires `llvm-config` to run. This is becoming more and more important as we write more FileCheck style tests (see Jonas' las

[Lldb-commits] [PATCH] D42955: Make Module::GetSectionList output consistent

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM. Comment at: source/Core/Module.cpp:1286 +if (SymbolVendor *vendor = GetSymbolVendor()) + vendor->CreateSections(*GetUnifiedSectionList()); } ---

[Lldb-commits] [PATCH] D43577: Fix TestUbsanBasic

davide added a comment. I wonder whether we could use something like >>> import os >>> os.path.basename('/patatino/ino/main.c') 'main.c' to make this slightly more robust against files which end in `main.c` but we don't want to really match, e.g. `blahmain.c`. https://reviews.llvm.org/D

[Lldb-commits] [PATCH] D43506: Fix a couple of more tests to not create files in the source tree

davide added a comment. LGTM. This is really good work, thanks https://reviews.llvm.org/D43506 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43471: Handle typeof() expressions

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D43471 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lld

[Lldb-commits] [PATCH] D43471: Handle typeof() expressions

davide added inline comments. Comment at: lit/Expr/TestTypeOfExpr.test:2 +# RUN: %lldb -b -s %s | FileCheck %s + +expression int i; __typeof__(i) j = 1; j I really really love how concise and clear the new test is! https://reviews.llvm.org/D43471 ___

[Lldb-commits] [PATCH] D43471: Handle typeof() expressions

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. Jonas, this looks a good use case for using lit. Is it possible to reuse the machinery we use in `lldb/lit/Expr` ? If not, well, we know there's something we can improve :) https://r

[Lldb-commits] [PATCH] D43464: Avoid dirtying the source tree in breakpoint command tests

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM, thanks for doing this :) https://reviews.llvm.org/D43464 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cg

[Lldb-commits] [PATCH] D43376: Fix TestStopReplyContainsThreadPcs on 32-bit x86 (pr36013)

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D43376 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lld

[Lldb-commits] [PATCH] D43292: [dotest] Add ability to skip tests based on build configuration

davide added a comment. After all the work he did on the testsuite I think Adrian is in a good position to review this one. https://reviews.llvm.org/D43292 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mail

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

davide added a comment. In https://reviews.llvm.org/D43048#1004807, @labath wrote: > (Btw, if you're looking for things to FileCheck-ify, I think the stuff under > `lldb/unittests/UnwindAssembly` is a prime candidate and has a much higher > bang/buck ratio.) If you have ideas on how to FileCh

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

davide added inline comments. Comment at: source/Target/Target.cpp:3949 + idx); + assert(option_value); + return option_value->GetCurrentValue(); aprantl wrote: > davide wrote: > > add an assertion m

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

davide accepted this revision. davide added a comment. LGTM modulo minor. Comment at: source/Plugins/ExpressionParser/Clang/CMakeLists.txt:26 clangCodeGen +clangDriver clangEdit aprantl wrote: > I checked and this does not affects LLDB's binary si

[Lldb-commits] [PATCH] D43059: Add implementation for MSVC in CPlusPlusLanguage::IsCPPMangledName

davide added a comment. Can you add a unittest for this? :) Repository: rL LLVM https://reviews.llvm.org/D43059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

davide added a comment. In https://reviews.llvm.org/D43048#1001311, @davide wrote: > In https://reviews.llvm.org/D43048#1001283, @zturner wrote: > > > By the way, I'd suggest printing indices in front of each match and > > including those in the FileCheck tests. Otherwise we could miss > > com

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

davide added a comment. In https://reviews.llvm.org/D43048#1001283, @zturner wrote: > By the way, I'd suggest printing indices in front of each match and including > those in the FileCheck tests. Otherwise we could miss completions that sneak > in. Instead, or in addition, we might dump the

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

davide added a comment. In https://reviews.llvm.org/D43048#1001293, @davide wrote: > In https://reviews.llvm.org/D43048#1001287, @jingham wrote: > > > The current auto-completer tests aren't interactive - they do exactly the > > same thing your command does, but from Python. It's fine if you wa

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

davide added a comment. In https://reviews.llvm.org/D43048#1001287, @jingham wrote: > The current auto-completer tests aren't interactive - they do exactly the > same thing your command does, but from Python. It's fine if you want to add > tests but please don't remove the current tests since

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

davide added a comment. You can take a look at the `test/testcases/functionalities/completion/TestCompletion.py` for the python equivalent. I find the potential FileCheck'ed version much easier to read/write/understand. I'm possibly biased having worked many years on LLVM, hence I'm asking for

[Lldb-commits] [PATCH] D43048: [lldb-test/WIP] Allow a way to test autocompletion

davide created this revision. davide added reviewers: aprantl, vsk, friss, labath, zturner, jingham, jasonmolenda. This is an experiment to improve out lldb testing capabilities and making them more similar to the one used in LLVM. Example: davide@Davidinos-Mac-Pro ~/w/l/b/bin> ./lldb-test a

[Lldb-commits] [PATCH] D43024: [test] Enable test category for inline tests.

davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. thanks for fixing this. Comment at: lldb/packages/Python/lldbsuite/test/decorators.py:526 return None - + header = os.path.join( ---

[Lldb-commits] [PATCH] D42914: Rewrite TestTargetSymbolsBuildidCase to be more focused

davide added a comment. In https://reviews.llvm.org/D42914#997974, @zturner wrote: > Yea this seems like a good candidate for an lldb-test test. Something like > this. > > RUN: yaml2obj %S/Inputs/stripped.yaml > %t.stripped.out > RUN: yaml2obj %S/Inputs/unstripped.yaml > > %t/.build-id/1b

[Lldb-commits] [PATCH] D42914: Rewrite TestTargetSymbolsBuildidCase to be more focused

davide added a comment. Nice :) This looks already fine as-is, but I wonder whether we can get rid of the python boilerplate altogether? There has been quite a bit of discussion about using `lldb-test` for this sort of more focused testing, so I wonder whether you gave it a try? (just a random

[Lldb-commits] [PATCH] D42912: Sync PlatformNetBSD.cpp with Linux

davide added a comment. LGTM. I always found supercumbersome having to check `if log()` and error prone. Repository: rL LLVM https://reviews.llvm.org/D42912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images

davide added a subscriber: zturner. davide added a comment. In https://reviews.llvm.org/D42870#996913, @krytarowski wrote: > Is there a working example of this? I would clone an existing code for Linux > or other supported OS and adapt it for NetBSD. > > Please note that I'm in the process of re

[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images

davide added a comment. In https://reviews.llvm.org/D42870#996899, @clayborg wrote: > Probably take a ELF file that is NetBSD and obj2yaml it. The test would run > yaml2obj on it and then test that things are recognized correctly via the SB > interfaces (check triple is correct)? The SBApi in

[Lldb-commits] [PATCH] D42870: Correct recognition of NetBSD images

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. Please add a test case. Repository: rL LLVM https://reviews.llvm.org/D42870 ___ lldb-commits mailing list lldb-commits@lists.llvm.or

[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

davide accepted this revision. davide added a subscriber: aprantl. davide added a comment. This revision is now accepted and ready to land. This looks good. Feel free to go ahead and commit, but please coordinate with @aprantl as he just landed his changes for the testsuite (so you might need to

[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

davide added a comment. In https://reviews.llvm.org/D42656#991284, @zturner wrote: > If we just need to test completion, write a lit-style test that uses > lldb-test that looks like this: > > RUN: lldb-test complete --target=%T/foo --complete_str=MyPrefix | FileCheck > %s > > CHECK: Foo::

[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

davide added a comment. In https://reviews.llvm.org/D42656#991239, @davide wrote: > In https://reviews.llvm.org/D42656#991065, @jingham wrote: > > > There are a whole bunch of other tests that test completion in this file > > that use the exact same mechanism but don't seem to be flakey. Why is

[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

davide added a comment. In https://reviews.llvm.org/D42656#991065, @jingham wrote: > There are a whole bunch of other tests that test completion in this file that > use the exact same mechanism but don't seem to be flakey. Why is this one > test flakey? So, I take a look at this to reply to

[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

davide added a comment. In https://reviews.llvm.org/D42656#991065, @jingham wrote: > There are a whole bunch of other tests that test completion in this file that > use the exact same mechanism but don't seem to be flakey. Why is this one > test flakey? > > If for instance it's because "Fo" en

[Lldb-commits] [PATCH] D42656: [testsuite] Remove flakey test relying on `pexpect`

davide added a comment. In https://reviews.llvm.org/D42656#991065, @jingham wrote: > There are a whole bunch of other tests that test completion in this file that > use the exact same mechanism but don't seem to be flakey. Why is this one > test flakey? > > If for instance it's because "Fo" en

[Lldb-commits] [PATCH] D42620: [lldb] Silence signed <-> unsigned integer comparison warning

davide accepted this revision. davide added a comment. LGTM. `unsigned` is probably fine as well. Do you need somebody to commit this on your behalf? https://reviews.llvm.org/D42620 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://l

[Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

davide added a comment. As there are no strong objections, I'm going to check this in tomorrow PST and see how it goes. Thanks for your contribution. https://reviews.llvm.org/D40283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://

[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

davide added a comment. In https://reviews.llvm.org/D42563#988700, @alexshap wrote: > @davide - the test case is in the description but i can try to add it to the > test suite. yes, please. Repository: rL LLVM https://reviews.llvm.org/D42563

[Lldb-commits] [PATCH] D42563: [lldb] attempt to fix DIERef::GetUID

davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. Testcase? Repository: rL LLVM https://reviews.llvm.org/D42563 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists

[Lldb-commits] [PATCH] D42488: Remove ObjectFile usage from HostLinux::GetProcessInfo

davide accepted this revision. davide added a comment. Thanks for the spring cleaning! https://reviews.llvm.org/D42488 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

davide added a comment. In https://reviews.llvm.org/D40283#983946, @aprantl wrote: > In https://reviews.llvm.org/D40283#983908, @clayborg wrote: > > > I am fine with checking this. The only issue on my end is the extra memory > > that will be needed to store these often huge mangled names in eve

<    1   2   3   4   5   6   >