- User Since
- Sep 23 2014, 5:24 PM (233 w, 6 d)
Wed, Mar 13
Also naming quibble...
I think you have to protect against your dyn_cast failing.
Tue, Mar 12
Mon, Mar 11
It's a little weird that the CommandObjectExpression is telling the REPL what its options should be, it would be more natural to go the other way. Maybe have the repl_sp provide a properly set up set of EvaluateExpressionOptions and then copy them over to this CommandObjectExpression execution?
Wed, Mar 6
I want to keep this commit simple so I'm leaving Adrian's substantial comments for a future commit.
I forgot to sort the project file before making the diff. I'll do that on commit.
Mon, Mar 4
Yes, looks fine.
Fri, Mar 1
This is fine by me, though Greg I think still had an outstanding question. If he's satisfied, I am.
Excellent, thanks! I didn't think to look there.
Wed, Feb 27
This seems reasonable to me. Any Host implementation can live off source/Host/common/Symbols.cpp, and only needs to do anything if it wants to augment that behavior. Seen that way, it is not defining API's that are a required step in porting to a new platform. So the API definitions don't need to be specially called out as being required of a Host implementation. And the implementation is still in source/Host so it's clear that this does provide host specific functionality. So that seems fine to me.
Tue, Feb 26
I chose not to do it that way because I don't think it will ever be the case the msgSend_stret will NOT be a struct return convention so the test isn't relevant (and is a bit confusing) there. And we may at some point need to do some other work that depends on the flavor of msgSend, so I wanted to keep the distinction.
I'm fine with leaving the reporting as is. This really only happens in fairly restricted situations (only hardware breakpoints) and neither way of reporting the failure seems much better to me, so we needn't over-polish it.
One comment seems to have wandered out of place.
I don't remember why this was inserted as separate pass. Maybe we were still early on learning what SWIG could do? That was a long time ago now...
I have two questions about this patch.
Mon, Feb 25
There aren't many more platforms (OpenVMS?) that are likely to show up and need support for lldb, so maybe this is making too much of the matter. But having there be a well defined set of places where you need to look when porting lldb to a new host platform seems a useful design to me. The fact that we weren't strict enough and allowed Host platform dependencies to creep in where they don't belong doesn't argue we should just abandon trying to keep the places where a port sockets into lldb regular.
Utility is supposed to be a bunch of stand-alone utility files & headers that we gather together for convenience's sake.
Fri, Feb 22
Thu, Feb 21
Is this the right way to do it? I don't know why the other was UNSUPPORTED: linux. The only other XFAIL for linux I could see spelled it -linux-?
Wed, Feb 20
Pavel, I think the new behavior w.r.t. launching is better. When the process plugins are managing events to handle a launch or attach, they really don't want random stop hooks getting in their way. On MacOS, if you set a stop hook before launch you always got one stop message at a place you didn't cause the target to stop, which I think was just confusing.
Trying to guess how thread list output is going to look is indeed a losing game.
Tue, Feb 19
I wasn't using the right %lldb when running the tests. That provides the 'echo-comment-commands false'. Run correctly you don't need the command in the test files.
Feb 15 2019
BTW, I still think that we should add an explicit way to indicate that stop hooks should continue. But the current intended behavior was regular. If any stop hook caused the target to proceed, it would terminate at that point and all the other stop hooks would be jettisoned. But if the stop hook gets (incorrectly) run in sync mode, we can no longer implement that policy.
Ah, I see what happened.
Feb 14 2019
I haven't gotten a chance to revisit the stop hooks in a long time. They definitely need some love.
That code seems amazingly good to me ;-)
Feb 13 2019
You are using a mix of llvm & lldb naming conventions for local variables and arguments and the ivars of UserIDResolver (lots of "Uid", etc...) Probably better to stick with the lldb convention.
Feb 11 2019
Feb 8 2019
Might be clearer to do:
Feb 7 2019
Feb 5 2019
Feb 4 2019
Thanks for adding the ObjC tests! I had two trivial requests for the test comments to be a little clearer, but even without that this is fine.
That looks good. Could you add a test for this setting to the ./functionalities/jitloader_gdb/TestJITLoaderGDB.py test? I wouldn't test that the default has any particular behavior because that might change over time. But test that if you turn it on, you do get load notifications, and if you turn it off you don't. Thanks!
It would be better to have the setting be an enum of "on/off/default", and then have the somebody - the current DynamicLoader plugin seems the best somebody - provide the default value if the setting hasn't been explicitly set. That way on any platform one could turn the loading on and off, which seems useful, and we wouldn't have to have a Darwin specific setting that will cease being applicable when the Darwin default switches.
Feb 1 2019
Sure, that's clearer.