- User Since
- Sep 23 2014, 5:24 PM (260 w, 5 d)
Fri, Sep 20
This is all about the decision in "batch" mode for what constitutes a stop by the program that should cause batch mode to return the control to the user.
Tue, Sep 17
This looks right.
Mon, Sep 16
Fri, Sep 13
Approved with that addition.
The help for -f should say what happens if this option isn't provided. Otherwise this looks fine.
Thu, Sep 12
I don't think CallNoArgNoReturnFunc is the right name. This routine calls a function that takes no arguments but returns a void *.
There's already Utils/StreamGDBRemote.cpp... It might be natural to stick the class in there?
It seems weird to me to have the GDBRemotePacket class live in the repro namespace. There's nothing particularly Reproducer specific about it, and it clearly gets used outside the reproducer as well. Wouldn't it make more sense to have this in Utils?
The code looks fine to me.
Also, since in C++ you can't do:
Your description says that if there are bits not belonging to the enum, you will print them after the enum values are listed. But you don't have a test for that. I thought you were going to use the "nonsense" var for that, but then you didn't...
Wed, Sep 11
If I've run lldb like:
This seems like the right place to put it, it's clearly something you would do with a process.
Tue, Sep 10
Mon, Sep 9
This pattern repeated a couple of times north or the one you fixed. Can you get those too?
Fri, Sep 6
Thu, Aug 29
That was the only test that tested the "command regex" functionality. It was probably called TestFormats.py because somebody copied over a test without changing the file name since that name makes no sense.
Does anybody know what this was supposed to test?
Wed, Aug 28
I use -# for running single tests multiple times. This is useful particularly if you have a test that only fails sometimes, you can do:
The whole command flags was a late addition, but we were using it for new commands and "when you touch it" kind of changes. That seems to have stalled the conversion, so thanks for completing this!
Aug 22 2019
You can go a long time without actually using the Dummy target, which is why I made it lazily.
Aug 19 2019
Aug 16 2019
Pavel said "we'll just have to bite the bullet and say that our expressions are now (more or less) POSIX conformant"... We should say this somewhere users are likely to find.
I don't think you need to go back and cover all the ones already added, but could we start adding tests for new lldb/llvm data formatters as we add them? That way we can keep them useful as people change llvm.
Aug 15 2019
Aug 14 2019
Does this seem clearer? We use the language of Private and Public StopInfo's in a bunch of places, even though at present that's more about "How" you get them than actual separate entities. But still, this seems like it follows the language we are using elsewhere.
Do you think I should put more verbiage in the comment, or would this have been clear if you were actually working on the code?
I think we need to fix the behavior for "", other than that, this looks fine to me.
Aug 13 2019
Thanks for pointing that out.
Fixed the inverted call order.
Aug 9 2019
Aug 8 2019
Aug 6 2019
Given that we do some work to find executables (like search along the path for "ls" -> /bin/ls") which work might not end up with the result you expected, printing the full path seems useful.
Thanks for doing this. Looks okay to me just one typo... But I don't know CMake well enough to comment on substance...
Aug 2 2019
That would be fine. In the swift REPL, we increment the line number so that it appears that all the expressions are one contiguous source file. But I think for the expression parser treating each expression as a separate file is a better fiction.
Aug 1 2019
Okay, if clang-format won't undo this, then I'm fine with writing them this way.
IIRC both GetPath and ResolvePath return the number of characters it would take to actually resolve the path. 128 is actually pretty short, so you should check the result and malloc a buffer of the return + 1 if 128 ends up not being enough.
If you just want the path and don't need the FileSpec, you can call the static SBFileSpec::ResolvePath.
What happens if you clang-format your reformatted version of the setter? Do we need clang-format off for them?
Jul 31 2019
If we don't want to forget that there was specific useful info, we could gate this patch on implementing the "reproducer generate <FEATURE>" feature. That should be easy to add if we're clear this this is the right design for reproducers. But I don't think that "bugreport unwind" was all that much used, so it might be better to design this at leisure.
The thing the "bugreport unwind" adds is that it runs a handful of commands that gather data useful in diagnosing unwind problems. That's useful when the information you need might not be output by the session in which the bug appeared.
Jul 29 2019
It might be good to see if you can trigger the bug more directly (for instance by getting the SBType for the method and pulling out its arguments?) The reason that a breakpoint triggers this at present is that lldb reads the debug info for the function to find the prologue extents so it can more accurately push the breakpoint past the prologue. But that's not really desirable, it is a lot of work just for setting a breakpoint, and if I ever figure out how not to do that, I will. In which case, this test will no longer test what you think it tests.
It worries me a little bit that we are making it harder and harder to figure out "where does the option for "-t" get stored once this CommandObject's options have been parsed. Can you show the steps I would have to go through to get from "-f" to OptionEnumSettingsSet::Force or whatever.
Jul 26 2019
This seems okay to me. I would also check with Jason. I don't know who coordinates running tests remotely - or even if they can run in parallel now. But that's the one bit I don't understand well enough to say yea or nay on.
It seems to me part of the goal of Alex's efforts is to make it possible for a given lldb to have or not have support for any particular type system. In some future day they might even be live pluggable, so that which ones get loaded would be a user choice. In that future, it is never an error to not have a given type system. And even if lldb has builtin code to support a given type system, I may not want to pay the cost to construct it. If I'm debugging ObjC code in a program that also supports swift, I might want to tell lldb not to bother with swift types.
Yeah, I didn't want to bother with formatting in case there were suggestions on content. I checked in a version that's correctly truncated.
Jul 25 2019
There isn't an SB API for "settings set" yet. I've put off doing that because the "settings" subsystem is currently unfinished. In the original design you would be specify qualifiers in the setting hierarchy, like:
Jul 24 2019
Sure, this is okay.
Jul 23 2019
I agree with Greg, having one function that can do any of the combinations of stdout & stderr seems more convenient.
Jul 22 2019
Actually, I don't want this change as is. Some logs - like the expression and step logs - are laid out for readability, and LLDB_LOG automatically adds the file & function which will make them much harder to read.
IIUC, LLDB_LOG already adds the file and function. A bunch of these logs are also adding FUNCTION, so that's probably going to come out twice now. You should probably remove the duplicates.
Jul 18 2019
This looks fine to me. Makes it really clear that we need SBTarget::FindFirstTypeForLanguage, etc. But FindFirstType was always a crapshoot anyway...
Jul 17 2019
This looks good in general. I also have a few nits, and you need to fix (and probably test) the behavior for files with spaces in their names.
I wonder if we ought to allow target properties (target as an example) that are only for testing, so they don't print when you do settings list, etc. But the we could have some settings like a "target.testing.dont-read-LC_MAIN" and that would make it easy to automate your hand testing. Kind of like the "experimental" decorator, except you have to know they exist to use them...
The nice thing about the way the ObjCLanguageRuntime::Get was that is was only useable where we decided it was legit to use it, in the actual ObjCLanguageRuntime plugin or its direct users. If you want to keep the convenience cast function in a header in Plugins/ExpressionParser/Clang, that would be fine.
Seems like in most places we're just pulling out basic types or their sizes, which we should certainly be able to do with a TypeSystem.