[Lldb-commits] [PATCH] D44072: [lldb] Retrieve currently handled Obj-C exception via __cxa_current_exception_type and add GetCurrentExceptionBacktrace SB ABI

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That's great. There are a couple places where you use self.assertEqual(..., False) where you can use self.assertFalse if you want. A very minor point... Comment at: pac

[Lldb-commits] [PATCH] D55954: [ldb] Add a "display-recognized-arguments" target setting to show recognized arguments by default

jingham added a comment. That's great. Can you add a test that if you make a default SBVariableOption and then flip the target setting you get the target's default value? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55954/new/ https://reviews.llvm.org/D55954

[Lldb-commits] [PATCH] D55954: [lldb] Add a "display-recognized-arguments" target setting to show recognized arguments by default

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Excellent! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55954/new/ https://reviews.llvm.org/D55954 ___ lldb-commits mailing list lldb

[Lldb-commits] [PATCH] D55318: [Expressions] Add support of expressions evaluation in some object's context

jingham added a comment. The only reservation I have is that this really ought to work correctly for ObjC as well as C++, and there's code to support that, but it isn't tested. I don't have time to write ObjC tests before the new year for sure. Is it possible to rope somebody else into that?

[Lldb-commits] [PATCH] D46733: Add a lock to PlatformPOSIX::DoLoadImage

jingham accepted this revision. jingham added a comment. That looks fine to me. https://reviews.llvm.org/D46733 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.

jingham added a comment. For #2 we should be able to come up with something much more trivial than pexpect (and thereby more reliable) because we really aren't expecting patterns. We are always sending a complete string reading a complete string back, then checking the string against some patt

[Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. The REPL is just a mode of the expression command. You invoke it by saying: (lldb) expr --repl -- or you can add any other options to it that you want, including flags like -i or

[Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

jingham added a comment. Perhaps a better way to handle this is to think of REPL.cpp/h as adjuncts of CommandObjectExpression.cpp/h - they mostly define the fancy IOHandler that gets pushed when you run the command in this mode. I think it would be fine to move them to Command, at which point

[Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

jingham added a subscriber: zturner. jingham added a comment. The expression command had two modes before introducing the REPL. You can do: (lldb) expr -- some C expression or you can do: (lldb) expr Enter expressions, then terminate with an empty line to evaluate: 1: first C expression 2

[Lldb-commits] [PATCH] D47302: [lldb, lldb-mi] Add method AddCurrentTargetSharedObjectPath to the SBDebugger.

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. Adding an SB API to set the target's search paths seems fine to me. I think in theory the totality of LLDB functionality should be available through the SB API's, (a) because it i

[Lldb-commits] [PATCH] D47232: Break dependency from Expression -> Commands

jingham added a comment. I worry when concerns of layering which seem a little artificial to me would make us add a shadow class for something that is already well expressed as it is. If you pass them as arguments, and then I add another useful one and want to use it in both the regular and th

[Lldb-commits] [PATCH] D47991: Add method SBThread::StepOver with SBError parameter.

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. Sorry for coming in late (WWDC...) It does seem asymmetric to only have one stepping API take an error. If one is going to they all should. You need to add the new API to the SBT

[Lldb-commits] [PATCH] D47991: Improve SBThread's stepping API using SBError parameter.

jingham added a comment. I agree with Leonard, for the StepOver overload that returns errors, just make the mode parameter required. That will reduce confusion. https://reviews.llvm.org/D47991 ___ lldb-commits mailing list lldb-commits@lists.llvm.

[Lldb-commits] [PATCH] D48114: Add dataformatter for NSDecimalNumber

jingham accepted this revision. jingham added a comment. This looks fine to me. Nothing in the TypeSummaryOptions is relevant to printing these numbers, so it's correct to ignore them. Repository: rL LLVM https://reviews.llvm.org/D48114 ___ lld

[Lldb-commits] [PATCH] D48049: Add a new SBTarget::LoadCore() overload which surfaces errors if the load fails

jingham added a comment. SBTarget.Attach, and Launch take SBError references, and they are pretty well tested. So I don't think that's a concern here. We don't use the text content of error messages programmatically in lldb. If you wanted to make these errors actionable, the low level LoadCore

[Lldb-commits] [PATCH] D48177: Suppress SIGSEGV on Android when the program will recover

jingham added a comment. If you can get the address of the bad access from the signal, you could also check that it was 0x0 and only suppress the SIGSEGV if it is? Also, do you want to put in a setting to turn this behavior off? If the code in any of the files of this type were to crash for so

[Lldb-commits] [PATCH] D44603: [lldb] Introduce StackFrameRecognizer

jingham added a comment. This is going as I imagined it should, looks great! We probably want to turn this on by default for "frame var" or no one will ever discover it. The IDE folks can decide on their own what to do from the SB API's. Since you are doing module filtering and this only trig

[Lldb-commits] [PATCH] D48177: Suppress SIGSEGV on Android when the program will recover

jingham added a subscriber: clayborg. jingham added a comment. Thanks for the explanation! Jim https://reviews.llvm.org/D48177 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44603: [lldb] Introduce StackFrameRecognizer

jingham added a comment. I think ultimately we should make this presentation of variables as symmetrical with locals as we can or it's going to confuse users. I don't think it is necessary for the first version. The first uses, I would imagine, are to make available some variables in situation

[Lldb-commits] [PATCH] D44603: [lldb] Introduce StackFrameRecognizer

jingham added a comment. This code looks fine to me. The docs are a little thin. You don't say how to make a recognizer anywhere except in the test example. There should probably be an example in the long help for CommandObjectFrameRecognizerAdd. Look for instance at the help for "breakpoint

[Lldb-commits] [PATCH] D47991: Improve SBThread's stepping API using SBError parameter.

jingham added a comment. The SB API's tend to take SBError as an in/out parameter for the cases where the function in question naturally returns some other value, and return an SBError when there's no other logical return value. So in these cases it would be more in line with the general practi

[Lldb-commits] [PATCH] D47991: Improve SBThread's stepping API using SBError parameter.

jingham added a comment. Won't this break client code that was calling StepOver? We are pretty serious about maintaining binary compatibility with the SB API's. Jim https://reviews.llvm.org/D47991 ___ lldb-commits mailing list lldb-commits@lists.

[Lldb-commits] [PATCH] D47991: Improve SBThread's stepping API using SBError parameter.

jingham added a comment. The client won't be expecting a struct return, and will have generated code to take a void return. If SBError happens to be returned in registers, nothing bad will happen, but if it's returned on the stack, that would corrupt the caller's stack. Relying on the particu

[Lldb-commits] [PATCH] D47991: Improve SBThread's stepping API using SBError parameter.

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That seems fine to me. https://reviews.llvm.org/D47991 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[Lldb-commits] [PATCH] D48450: [DataFormatter] Add CFDictionary data formatter

jingham added subscribers: JDevlieghere, jingham. jingham added a comment. Yes, I really wish unittest2 allowed non-aborting tests. If you split up all these tests, then you make the testsuite have to build and run some little executable over and over. It would be so nice if there were a way t

[Lldb-commits] [PATCH] D48463: Prevent dead locking when calling PrintAsync

jingham added a reviewer: clayborg. jingham added a comment. The IOHandlers are Greg's construct. He's likely the best person to review this. https://reviews.llvm.org/D48463 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.ll

[Lldb-commits] [PATCH] D48659: Allow specifying an exit code for the 'quit' command

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This won't at all do what users expect in cases where the driver program doesn't cooperate. For instance, when run under Xcode, the lldb process can have many debuggers running co

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

jingham added a comment. This behavior change seems desirable. But there are other commands that use the IOHandlerActivated method to print instructions like this, and should get the same treatment (breakpoint command add, type summary add...). It seems ugly to have to do this one by one in i

[Lldb-commits] [PATCH] D48752: Quiet command regex instructions during batch execution

jingham added a subscriber: kastiglione. jingham added a comment. Sure, that also sounds fine. Jim https://reviews.llvm.org/D48752 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48796: Refactoring for for the internal command line completion API (NFC)

jingham added a comment. This patch looks fine, I agree with Pavel the direction is good. It seems like you would reduce a bunch of boiler-plate if you changed over InvokeCommonCompletionCallbacks to take a CompletionRequest. Was there something that blocked you from doing that? https://rev

[Lldb-commits] [PATCH] D48796: Refactoring for for the internal command line completion API (NFC)

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Okay, sounds reasonable. https://reviews.llvm.org/D48796 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[Lldb-commits] [PATCH] D48801: Add new API to SBTarget and SBModule classes.

jingham added a comment. Alex is right, lldb uses @param everywhere. https://reviews.llvm.org/D48801 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48865: [LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID

jingham added a comment. This is correct, thanks for catching it! There are a few other places that use m_thread_idx above this change. Everything after the: if (thread == nullptr) { check should also use thread->GetID not m_options.m_thread_idx. This is all error reporting, so it's not a

[Lldb-commits] [PATCH] D48865: [LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID

jingham added a comment. Sorry, I wasn't clear. Suppose that the `m_thread_idx` coming into DoExecute is LLDB_INVALID_THREAD_ID. That's what happens if you don't select a thread id on the command line, for instance. Then the code around 1164 will set `thread` to be the default thread. But `

[Lldb-commits] [PATCH] D48659: Allow specifying an exit code for the 'quit' command

jingham accepted this revision. jingham added a comment. That seems reasonable to me. https://reviews.llvm.org/D48659 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D44603: [lldb] Introduce StackFrameRecognizer

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This is good. I had a few inline comments, mostly about the command syntax. I think you should switch "-m" to "-s" since that's what we use in the other similar places. For the

[Lldb-commits] [PATCH] D49106: Refactor parsing of argument lists with a raw string suffix.

jingham added a comment. This is great. The only quibble I have is that I would really like to restrict ArgsWithSuffix to be "OptionsWithSuffix". I don't think we want to add: (lldb) some_cmd -f 0 -g 0 SomeArg OtherArg -- raw string That seems unnecessarily general, and not terribly helpful.

[Lldb-commits] [PATCH] D49106: Refactor parsing of option lists with a raw string suffix.

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That looks good. https://reviews.llvm.org/D49106 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[Lldb-commits] [PATCH] D49207: Get rid of the C-string parameter in DoExecute

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. I think the CommandObjectRegex check that you took out wasn't something devious or clever, but was just an incomplete (and so buggy) version of the check: if (command && command[0]

[Lldb-commits] [PATCH] D48865: [LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Thanks. https://reviews.llvm.org/D48865 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

[Lldb-commits] [PATCH] D49207: Get rid of the C-string parameter in DoExecute

jingham added a comment. Yeah, you are right about that. The command string presented to commands never contained the command name, that always got stripped off. But I don't think that ever left the command string to be nullptr... So you are right, that was just dead code. https://reviews.

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

jingham requested changes to this revision. jingham added a comment. It seems a little odd to choose to represent this as an array with 0 elements. I think I would be confused by that. You can maybe choose a better name like "value" for your cloned object. But it looks like it is also possibl

[Lldb-commits] [PATCH] D48976: Replaced more boilerplate code with CompletionRequest (NFC)

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Oops, sorry for the delay. This is so much easier to read. Thanks for cleaning this up! https://reviews.llvm.org/D48976 ___ lldb-commits mai

[Lldb-commits] [PATCH] D49311: Add includes for CompletionRequest to every file that uses it

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That works for me, thanks for fixing this. https://reviews.llvm.org/D49311 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lis

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

jingham accepted this revision. jingham added a comment. This looks fine. I agree with Davide that putting some description of the type you are formatting in comments somewhere would be make things easier for somebody else who might have to fix this (or to you when you've totally forgotten how

[Lldb-commits] [PATCH] D48351: Move dumping code out of RegisterValue class

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. I don't agree that moving dump routines away from the class that they are dumping is a good idea in general. It makes it harder for somebody new to the code to find out how to print out the

[Lldb-commits] [PATCH] D48351: Move dumping code out of RegisterValue class

jingham added a comment. Dump is really meant to be the private description of the object that you would use for logging and the like - Description was the public face of a class. So while the Description-like functionality could have a restrictions that constrain its use to pretty high up in

[Lldb-commits] [PATCH] D49949: Add missing boundary checks to variable completion.

jingham added a comment. One trivial fix to the test case. Repository: rL LLVM https://reviews.llvm.org/D49949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49740: Move RegisterValue, Scalar, State from Core to Utility

jingham added a comment. I worry a little bit about Utility getting kind of incoherent. Some of it is clearly utility, some is more there because that's where we are putting the code we're trying to reduce dependencies on so we can share then with lldb-server. RegisterValue doesn't for instan

[Lldb-commits] [PATCH] D49740: Move RegisterValue, Scalar, State from Core to Utility

jingham added a comment. First off, I'm fine with Zachary's suggestion that we let the dust settle a bit before we try to organize things better. We'll probably do a better job then. But just to use these cases, for instance, Scalar is the base of how we realize values in the debugger (ValueOb

[Lldb-commits] [PATCH] D50304: [lldb] CommandObjectThreadUntil should set breakpoint at either on exact or the nearest subsequent line number but not on all the subsequent line numbers

jingham added a comment. This implementation forces an extra lookup when the line number slides. For instance, if there were only one line match, but it was not exact, you're going to look up the exact address, fail, then look it up with exact = false. Wouldn't it be more efficient to look wit

[Lldb-commits] [PATCH] D49740: Move RegisterValue, Scalar, State from Core to Utility

jingham added a comment. In https://reviews.llvm.org/D49740#1188587, @labath wrote: > In https://reviews.llvm.org/D49740#1188055, @jingham wrote: > > > First off, I'm fine with Zachary's suggestion that we let the dust settle a > > bit before we try to organize things better. We'll probably do

[Lldb-commits] [PATCH] D50304: [lldb] CommandObjectThreadUntil should set breakpoint at either on exact or the nearest subsequent line number but not on all the subsequent line numbers

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That looks good. Thanks for cleaning this up! https://reviews.llvm.org/D50304 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http:/

[Lldb-commits] [PATCH] D49739: Add new API to SBTarget class

jingham added a comment. Could you add a short doc to this API - it's particularly useful to do this to the .i version. I know there are lots of API's that don't have docs, but that is not a happy situation. I'd like it if we all at least didn't make the problem worse by documenting all the n

[Lldb-commits] [PATCH] D49739: Add new API to SBTarget class

jingham added a comment. Thanks! No need for review... Repository: rL LLVM https://reviews.llvm.org/D49739 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D50071: Use rich mangling information in Symtab::InitNameIndexes()

jingham added a comment. You and Pavel have done great work getting this all straight. I don't have much to add except some trivial quibbles about variable naming, a grammar error and some other inessentials. Comment at: include/lldb/Core/Mangled.h:323-325 + /// @return +

[Lldb-commits] [PATCH] D50071: Use rich mangling information in Symtab::InitNameIndexes()

jingham accepted this revision. jingham added a comment. Looks good. https://reviews.llvm.org/D50071 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49739: Add new API to SBTarget class

jingham added a subscriber: apolyakov. jingham added a comment. It looks like lots of options to debugserver were added without also adding them to the --help output. Probably because there are so few clients they already know the options (the same person added & used them...) But if you look

[Lldb-commits] [PATCH] D50536: Fix: ConstString::GetConstCStringAndSetMangledCounterPart() should update the value if the key exists already

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This looks fine to me. https://reviews.llvm.org/D50536 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push the ProcessIO handler.

jingham added a comment. It isn't true that the ProcessIO handler will have a short duration, in general. It will live on the stack as long as process is running, which is of arbitrary duration. I actually don't think that much matters to this discussion, however. The most common IOHandler

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

jingham added a comment. For the most part this is fine. There are two bits to work on: 1. I think you could do all of this work on a static std::function object from the data section even if you don't have a process. It might be worth seeing whether you can do that. It looks like you can ma

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

jingham added a comment. Sounds like std::function objects need to run static initializers before they are useful. So trying to format them w/o a process won't do any good. Probably better to just return false directly if you don't have a process in that case. https://reviews.llvm.org/D508

[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.

jingham added a comment. m_last_natural_stop_id is the stop ID for the last time we stopped when the user was just running the process (run, continue, next, etc...) m_last_user_expression_resume is the resume ID for the last user expression resume. The stops and resumes always occur in pairs i

[Lldb-commits] [PATCH] D50864: Add libc++ data formatter for std::function

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This looks fine. There was one place where you referred back to case 1 by name not ordinal, but otherwise this is quite clear. Don't need another round of review, just fix that nit and its

[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.

jingham added a comment. What would happen to any output that the process produced while running the utility function if we did this. https://reviews.llvm.org/D50912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cg

[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.

jingham added a comment. It's not a huge deal, but it would be disconcerting to have a bunch of text dumped to your console the next time you continue. I think the ideal solution would be for utility functions to push a "capture & report" IO handler, so the Utility function could retrieve what

[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump

jingham added a comment. That's not changing the module list, that's changing the loaded section information. It is the dynamic loader's job to figure out what got loaded where, plus your stack trace show this happening after we've selected a plugin, not in the process of negotiating for the p

[Lldb-commits] [PATCH] D51176: Restrict the set of plugins used for ProcessMinidump

jingham added a comment. The other option would be to move the code that populates the section load list into the mini dump dynamic loader. That has the benefit of keeping this consistent with the other process plugins, but OTOH is just moving code around... Repository: rLLDB LLDB https:/

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

jingham added a comment. I wonder if it is worth asserting if a process ever returns a state of kNumStateType? I don't think it is necessary in things like StateIsStoppedState, somebody might have found their way here from the SB API's and we want to be forgiving there. But wherever we've don

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

jingham added a comment. Other than that, this looks fine to me. https://reviews.llvm.org/D51445 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

jingham added a comment. By name breakpoints don't use this function so a function regex wouldn't show the change. But the source regex does use this filter, so something like: (lldb) break set -p ; on a large source file might do it. That's a pretty silly thing to do, however. The only act

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

jingham added a comment. Except for the nit about the comment, this looks fine. https://reviews.llvm.org/D51453 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Oh, yeah, gotta remember to check the box... https://reviews.llvm.org/D51453 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://l

[Lldb-commits] [PATCH] D50912: Don't cancel the current IOHandler when we push a handler for an utility function run.

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This is fine by me. https://reviews.llvm.org/D50912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[Lldb-commits] [PATCH] D48465: Added initial code completion support for the `expr` command

jingham added a comment. Yes, this is fine. Thanks for working on this! https://reviews.llvm.org/D48465 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D51461: Support setting a breakpoint bile FileSpec+Line+Column in the SBAPI.

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This looks great, just a few small nits and a test request. Comment at: include/lldb/Breakpoint/BreakpointResolverFileLine.h:71 + uint32_t m_column; ///< This is

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

jingham added a comment. I'm not much surprised by that, but thanks for the experiments. As you say, that wasn't the primary purpose of this patch. Repository: rL LLVM https://reviews.llvm.org/D51453 ___ lldb-commits mailing list lldb-commits@l

[Lldb-commits] [PATCH] D51461: Support setting a breakpoint by FileSpec+Line+Column in the SBAPI.

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That's great, thanks! https://reviews.llvm.org/D51461 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/m

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

jingham added a comment. In https://reviews.llvm.org/D51445#1220905, @aprantl wrote: > Thanks! I assume there is a whole bunch of other enums for which we should be > doing the same thing now? > Also there might be some 1-based ones for which we also need to check the > lower bound. Be caref

[Lldb-commits] [PATCH] D51816: Fix raw address breakpoints not resolving

jingham accepted this revision as: jingham. jingham added a comment. This revision is now accepted and ready to land. Looks good to me. https://reviews.llvm.org/D51816 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/c

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

jingham created this revision. Herald added subscribers: lldb-commits, teemperor, abidh, jfb, srhines. This change allows you to make a breakpoint resolver kernel in Python. The breakpoint search mechanism in lldb works on top of a generic mechanism that uses a pair of Searcher - with its associ

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

jingham added a comment. Thanks for adding Greg. I am the code owner for the breakpoints part of lldb, and most recently I've been the only one adding to the scripting interface. There wasn't another really germane reviewer, and since the dev list was CC'ed I figured if anybody was interested

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. A bunch of little comments and two more substantial bits. 1. Can you add a test where we do "frame variable" on a one of these variants when it has one value, and then continue to

[Lldb-commits] [PATCH] D51896: Refactoring std::function formatter to move core functionality into CPPLanguageRuntime

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Since the runtime has to use the results of the introspection that the formatter is based on, it seems appropriate to put it there. This seems like a fine way to do it. https://reviews.ll

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

jingham added a comment. I addressed the individual concerns in the comments. There were a couple of overall points: 1. Could we avoid adding AddLocation by having the callback return a list of addresses to set breakpoints. I actually like directly saying "Set me a location there" and I have

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

jingham added a comment. I addressed most of the comments. I remember we argued about how the filters should work w.r.t. the resolvers back when I first did this, but I still claim my way is right ;-) I gave some arguments for that in response to your inline comment. I'm going to save this (

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

jingham updated this revision to Diff 165184. jingham added a comment. Add documentation for SBTarget.CreateBreakpointFromScript, and fixed a few minor nits that Greg pointed out. https://reviews.llvm.org/D51830 Files: include/lldb/API/SBAddress.h include/lldb/API/SBBreakpoint.h include/

[Lldb-commits] [PATCH] D51175: Add support for descriptions with command completions.

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This looks good. I agree with Jonas, however that we might have cases where we end up with two identical completions that have different descriptions, and we shouldn't drop them.

[Lldb-commits] [PATCH] D51175: Add support for descriptions with command completions.

jingham accepted this revision. jingham added inline comments. This revision is now accepted and ready to land. Comment at: source/Utility/CompletionRequest.cpp:79 + // Add the completion if we haven't seen the same value before. + if (m_added_values.insert(r.GetUniqueKey()).se

[Lldb-commits] [PATCH] D51520: Add libc++ data formatter for std::variant

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Yes, that's a good way to do it. https://reviews.llvm.org/D51520 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.or

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

This revision was automatically updated to reflect the committed changes. Closed by commit rL342185: Add a "scripted" breakpoint type to lldb. (authored by jingham, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D51830?vs=165184&id=165

[Lldb-commits] [PATCH] D51830: Add a way to make scripted breakpoints

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB342185: Add a "scripted" breakpoint type to lldb. (authored by jingham, committed by ). Changed prior to commit: https://reviews.llvm.org/D51830?vs=165184&id=165386#toc Repository: rLLDB LLDB htt

[Lldb-commits] [PATCH] D52065: WWW docs for scripted breakpoint resolvers

jingham created this revision. jingham added a reviewer: clayborg. Herald added subscribers: lldb-commits, abidh. These are the promised docs for https://reviews.llvm.org/D51830. Repository: rLLDB LLDB https://reviews.llvm.org/D52065 Files: www/python-reference.html Index: www/python-refe

[Lldb-commits] [PATCH] D51387: Allow Template argument accessors to automatically unwrap parameter packs

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. I'm a little worried that we have asserts with no backstops here. We ship in general with asserts off, so if some type is not copasetic we will probably crash shortly afterwards (

[Lldb-commits] [PATCH] D52111: Make eSearchDepthFunction work for scripted resolvers

jingham created this revision. jingham added a reviewer: clayborg. Herald added a subscriber: lldb-commits. I hadn't needed eSearchDepthFunction for anything so it was stubbed out. Greg indicated he would be interested in using this for the scripted resolvers, so I got it to work with this patc

[Lldb-commits] [PATCH] D52111: Make eSearchDepthFunction work for scripted resolvers

This revision was automatically updated to reflect the committed changes. Closed by commit rL342259: Make the eSearchDepthFunction searches work, add tests (authored by jingham, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52111?vs

[Lldb-commits] [PATCH] D52065: WWW docs for scripted breakpoint resolvers

jingham updated this revision to Diff 165614. jingham added a comment. I think this should address your concerns, though I did so more by reworking the presentation, so you won't find inserts exactly at the places you pointed out. But I tried to make it clear that you only write the Resolver, a

[Lldb-commits] [PATCH] D52065: WWW docs for scripted breakpoint resolvers

jingham marked 3 inline comments as done. jingham added a comment. I used your suggestion and fixed the few other bits. Comment at: www/python-reference.html:325 +Another use of the Python API's in lldb is to create a custom breakpoint resolver. This facility

[Lldb-commits] [PATCH] D52065: WWW docs for scripted breakpoint resolvers

jingham updated this revision to Diff 165796. jingham marked 3 inline comments as done. jingham added a comment. Addressed review comments from Greg. Repository: rLLDB LLDB https://reviews.llvm.org/D52065 Files: www/python-reference.html Index: www/python-reference.html ==

[Lldb-commits] [PATCH] D52065: WWW docs for scripted breakpoint resolvers

This revision was automatically updated to reflect the committed changes. Closed by commit rL342419: Add docs for scripted breakpoint resolvers (authored by jingham, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52065?vs=165796&id=16

<    1   2   3   4   5   6   7   8   9   10   >