Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

> On Sep 20, 2017, at 4:16 PM, Leonard Mosescu wrote: > > I don't quite understand the comment about signals adding indeterminacy. No > signal delivery is required to test this part. The lldb driver has a sigint > handler that calls SBDebugger::DispatchInputInterrupt. But since you aren't >

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Yes, using line endings as split points is somewhat arbitrary, anything that's a reasonable compromise between interruption responsiveness and low interrupt polling overhead would do. I feel that the lines are somewhat nicer since arbitrary splitting may lead to confusion and/or formatting ugliness

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

> > I don't quite understand the comment about signals adding indeterminacy. > No signal delivery is required to test this part. The lldb driver has a > sigint handler that calls SBDebugger::DispatchInputInterrupt. But since > you aren't testing whether SIGINT actually calls DispatchInputInterrup

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

> On Sep 20, 2017, at 11:25 AM, Zachary Turner wrote: > > > > On Wed, Sep 20, 2017 at 11:14 AM Jim Ingham > wrote: > > The amount of test coverage lldb has at present has much more to do with the > very aggressive schedules lldb has been driven by since its incepti

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

On Wed, Sep 20, 2017 at 11:14 AM Jim Ingham wrote: > > The amount of test coverage lldb has at present has much more to do with > the very aggressive schedules lldb has been driven by since its inception > than any difficulties with writing tests IMHBSEO. > This probably applies to the folks at

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

I write lots of tests. I don’t really think that the SB API’s are really a barrier to writing tests. The lldbinline tests are trivial to write and can even be made just as a transcription of an lldb command line session, so the barrier to entry is trivial. That is surely the easiest way to wr

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

On Wed, Sep 20, 2017 at 10:46 AM Jim Ingham wrote: > Directly WRT testing. I’m not against ALSO adding gtests when you add > some functionality. But when your change is actually adding behaviors to > lldb, one of the things you need to ask yourself is if this is > functionality that extension d

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Directly WRT testing. I’m not against ALSO adding gtests when you add some functionality. But when your change is actually adding behaviors to lldb, one of the things you need to ask yourself is if this is functionality that extension developers for lldb will benefit from. If it is, then addi

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

On Wed, Sep 20, 2017 at 10:33 AM Jim Ingham wrote: > One of the fundamental goals of lldb is that it be an extensible > debugger. The extension mechanism for command line lldb all runs through > the SB API through either Python or C++ (though most folks choose to use > Python). We know first ha

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

One of the fundamental goals of lldb is that it be an extensible debugger. The extension mechanism for command line lldb all runs through the SB API through either Python or C++ (though most folks choose to use Python). We know first hand that many people take advantage this mechanism to add c

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

On Tue, Sep 19, 2017 at 7:12 PM Jason Molenda wrote: > > > > On Sep 19, 2017, at 6:57 PM, Zachary Turner via lldb-commits < > lldb-commits@lists.llvm.org> wrote: > > > > > > > > After some additional thought, I stilled think testing below the sb api > layer is more appropriate. > > While Xcode

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

> On Sep 19, 2017, at 6:57 PM, Zachary Turner via lldb-commits > wrote: > > > > After some additional thought, I stilled think testing below the sb api layer > is more appropriate. > While Xcode might be going through the SB api, users of upstream lldb > (which is what we supposed to be

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

On Tue, Sep 19, 2017 at 11:51 AM Zachary Turner wrote: > On Tue, Sep 19, 2017 at 11:40 AM Jim Ingham wrote: > >> I must be missing something. >> >> DisaptchInputInterrupt knows how to interrupt a running process, and >> because of Enrico's Python interruption stuff it will interrupt script >> ex

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Here's a short description of the coding rules and the general idea of how to add API's: http://lldb.llvm.org/SB-api-coding-rules.html If you come across something you wished would have been in this document while you are implementing this, please add it. It's sometimes hard to see some cruci

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Any pointers on the steps required to make changes to the SB APIs? Is it documented anywhere? Thanks! On Tue, Sep 19, 2017 at 11:58 AM, Jim Ingham wrote: > > > On Sep 19, 2017, at 11:25 AM, Leonard Mosescu > wrote: > > > > These are all great suggestions, thanks everyone! > > > > > We should ha

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Thanks for the context. On Tue, Sep 19, 2017 at 11:58 AM, Jim Ingham wrote: > > > On Sep 19, 2017, at 11:25 AM, Leonard Mosescu > wrote: > > > > These are all great suggestions, thanks everyone! > > > > > We should have a test. The test would need to use pexpect or something > similar. If anyon

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

> On Sep 19, 2017, at 11:25 AM, Leonard Mosescu wrote: > > These are all great suggestions, thanks everyone! > > > We should have a test. The test would need to use pexpect or something > > similar. If anyone has any pointer on how to make a test for this, please > > chime in. I was thinking

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

On Tue, Sep 19, 2017 at 11:40 AM Jim Ingham wrote: > I must be missing something. > > DisaptchInputInterrupt knows how to interrupt a running process, and > because of Enrico's Python interruption stuff it will interrupt script > execution at some point but it wasn't very reliable last time I tri

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

That seems fine to me. Jim > On Sep 19, 2017, at 11:39 AM, Leonard Mosescu wrote: > > So, how about I look into exposing WasInterrupted() through SB APIs in a > follow up change? > > On Tue, Sep 19, 2017 at 11:36 AM, Jim Ingham wrote: > Xcode does, I don't know about other UI's. > > Jim >

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

I must be missing something. DisaptchInputInterrupt knows how to interrupt a running process, and because of Enrico's Python interruption stuff it will interrupt script execution at some point but it wasn't very reliable last time I tried it. But the whole point of this patch is that in gen

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

So, how about I look into exposing WasInterrupted() through SB APIs in a follow up change? On Tue, Sep 19, 2017 at 11:36 AM, Jim Ingham wrote: > Xcode does, I don't know about other UI's. > > Jim > > > On Sep 19, 2017, at 11:35 AM, Leonard Mosescu > wrote: > > > > I agree Jim. I'd like like to

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

On Tue, Sep 19, 2017 at 11:34 AM Jim Ingham wrote: > > > On Sep 19, 2017, at 11:30 AM, Zachary Turner wrote: > > > > > > > > On Tue, Sep 19, 2017 at 11:27 AM Jim Ingham wrote: > > We agreed to forwards compatibility because people write big scripts > that use the SB API, implement GUI's on top

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Xcode does, I don't know about other UI's. Jim > On Sep 19, 2017, at 11:35 AM, Leonard Mosescu wrote: > > I agree Jim. I'd like like to build thing incrementally - checking in the > current change as is does not preclude adding the SB APIs while it does > provide the foundation. > > I think

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

IIRC Enrico put in something where we would tell Python to interrupt at points where Python checks for interruptibility, but that is pretty herky-jerky. It would be much better to have the commands control this. Jim > On Sep 19, 2017, at 11:34 AM, Jim Ingham wrote: > > >> On Sep 19, 2017, a

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

I agree Jim. I'd like like to build thing incrementally - checking in the current change as is does not preclude adding the SB APIs while it does provide the foundation. I think that going after the scenarios you mention is a significant increase in scope. Are people even hooking up DispatchInterr

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

> On Sep 19, 2017, at 11:30 AM, Zachary Turner wrote: > > > > On Tue, Sep 19, 2017 at 11:27 AM Jim Ingham wrote: > We agreed to forwards compatibility because people write big scripts that use > the SB API, implement GUI's on top of them (more than just Xcode) etc. So we > try not to jerk

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

On Tue, Sep 19, 2017 at 11:27 AM Jim Ingham wrote: > We agreed to forwards compatibility because people write big scripts that > use the SB API, implement GUI's on top of them (more than just Xcode) etc. > So we try not to jerk those folks around. That adds a little more > responsibility on our

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Right, I can fix that. Give me a few minutes though. On Tue, Sep 19, 2017 at 11:28 AM Leonard Mosescu wrote: > This looks beautiful indeed. The problem is that it doesn't quite work > with the current MemoryBuffer and the line_iterator : for one thing there's > no way to construct a MemoryBuffe

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

This looks beautiful indeed. The problem is that it doesn't quite work with the current MemoryBuffer and the line_iterator : for one thing there's no way to construct a MemoryBuffer from a StringRef, or to use the line_iterator directly with a StringRef. On Tue, Sep 19, 2017 at 10:39 AM, Zachary

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

We agreed to forwards compatibility because people write big scripts that use the SB API, implement GUI's on top of them (more than just Xcode) etc. So we try not to jerk those folks around. That adds a little more responsibility on our part to think carefully about what we add, but the notion

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

These are all great suggestions, thanks everyone! > We should have a test. The test would need to use pexpect or something similar. If anyone has any pointer on how to make a test for this, please chime in. I was thinking just a pexpect based test. I love tests but what exactly do we want to test

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

I disagree. The things we care about are (a) that this works in C++ implemented commands and (b) that it works in Python commands. This doesn't seem to test either of those things. Also, I find writing tests for new functionality to be a great way to inform you about what you need to add to t

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Also, it avoids polluting the SB interface with another function that probably nobody is ever going to use outside of testing. Adding to the SB API should take an act of God, given that once it gets added it has to stay until the end of time. On Tue, Sep 19, 2017 at 11:15 AM Zachary Turner wrote

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

That would work, but I think it's adding many more pieces to the test. Now there's threads, a Python layer, and multiprocess dotest infrastructure in the equation. Each providing an additional source of flakiness and instability. If all it is is a pass-through, then just calling the function it

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

I'd really prefer to do this as/along with an SB API test since we also need commands made through the SB API to be interruptible, and we should test that that also works. So this kills two birds with one stone. In general, when developing any high-level features in lldb one of the questions y

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Python based commands will need to be able to call WasInterrupted if they are going to do their jobs. So it would make sense to add an SBCommandInterpreter::WasInterrupted API and make sure that works. And you can already send the interrupt with DispatchInputInterrupt. So it should be pretty

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Hi Greg, are you referring to the manual line splitting vs. using StringRef::split()? This is how it's documented: /// Split into two substrings around the first occurrence of a separator /// character. /// /// If \p Separator is in the string, then the result is a pair (LHS, RHS) /// such that (*

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

Looking at line_iterator, it seems to be designed to work with MemoryBuffer, which in turn seems specialized for dealing with file content (so while it may be possible to force init a MemoryBuffer from a StringRef it seems a bit awkward to me). Also, line_iterator has extra stuff which we don't nee

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

On Mon, Sep 18, 2017 at 3:13 PM Leonard Mosescu wrote: > It's a good question - here's a two part answer: > > 1. The actual printing of the output is the longest blocking in many cases > (as mentioned in the description: the actual data gathering for "target > module dump symtab" can take 1-2sec,

Re: [Lldb-commits] [PATCH] D37923: Implement interactive command interruption

It's a good question - here's a two part answer: 1. The actual printing of the output is the longest blocking in many cases (as mentioned in the description: the actual data gathering for "target module dump symtab" can take 1-2sec, but printing it can take 20min. For quick experiment, try dis -p