[Lldb-commits] [PATCH] D66241: stop-hooks don't fire on "step-out"

jingham updated this revision to Diff 215280. jingham added a comment. 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 fol

[Lldb-commits] [PATCH] D66241: stop-hooks don't fire on "step-out"

jingham added a comment. In D66241#1631426 , @clayborg wrote: > Better. Might be good to put a comment inside CalculatePublicStopInfo() > regarding why the "SetStopInfo(GetStopInfo());" is needed. Seems like you > could just remove the code from the sigh

[Lldb-commits] [PATCH] D66241: stop-hooks don't fire on "step-out"

This revision was automatically updated to reflect the committed changes. Closed by commit rL369052: Stop-hooks weren't getting called on step-out. Fix that. (authored by jingham, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: htt

[Lldb-commits] [PATCH] D66174: [Utility] Reimplement RegularExpression on top of llvm::Regex

jingham accepted this revision. jingham added a comment. 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. All the options that take regular expressions look like: (ll

[Lldb-commits] [PATCH] D66345: [lldb][NFC] Allow for-range iterating over StringList

jingham added a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66345/new/ https://reviews.llvm.org/D66345 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D66581: [lldb] Construct the dummy target when the first Dummy object is constructed

jingham accepted this revision. jingham added a comment. You can go a long time without actually using the Dummy target, which is why I made it lazily. Thinking of lldb as the command line program, we tend to think "I'll only ever make one debugger" so making one extra per-debugger thing doesn'

[Lldb-commits] [PATCH] D66628: [Symbol] Decouple clang from DeclVendor

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66628/new/ https://reviews.llvm.org/D66628 _

[Lldb-commits] [PATCH] D66863: [lldb] Unify target checking in CommandObject

jingham added a comment. Herald added a subscriber: JDevlieghere. 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! As a minor style thing, prior

[Lldb-commits] [PATCH] D66908: [dotest] Remove the -# option

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. I use -# for running single tests multiple times. This is useful particularly if you have a test that only fails sometimes, you can do: $ lldb-dotest -p testname.py -d -#400 th

[Lldb-commits] [PATCH] D66962: [lldb][NFC] Remove TestFormats.py as is tests nothing

jingham added a comment. Does anybody know what this was supposed to test? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66962/new/ https://reviews.llvm.org/D66962 ___ lldb-commits mailing list lldb-commits@lists.l

[Lldb-commits] [PATCH] D66962: [lldb][NFC] Remove TestFormats.py as is tests nothing

jingham added a comment. 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. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https:/

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

jingham added inline comments. Comment at: lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:901-902 // Make sure we deallocate the buffer memory: -buffer_cleanup.emplace([process, buffer_addr] { -process->DeallocateMemory(buffer_addr); -}); +buffer_

[Lldb-commits] [PATCH] D67378: [Utility] Replace Cleanup by llvm::scope_exit

jingham added a comment. This pattern repeated a couple of times north or the one you fixed. Can you get those too? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67378/new/ https://reviews.llvm.org/D67378 ___ lldb-commits mailing list lld

[Lldb-commits] [PATCH] D67472: [Target] Move InferiorCall to Process

jingham added a comment. This seems like the right place to put it, it's clearly something you would do with a process. I don't see any reason why this should be a private function. It's just a convenience wrapper around a bunch of other public functionality, and doesn't do anything that need

[Lldb-commits] [PATCH] D67474: [Reproducer] Add `reproducer dump` command

jingham added a comment. If I've run lldb like: $ lldb -capture-path and then I want to dump the reproducer I've loaded, it seems annoying to have to say: (lldb) reproducer dump -f Why do I need to type it again? So it seems like you could make -f optional, and if it isn't provided use th

[Lldb-commits] [PATCH] D67520: Add pretty printing of Clang "bitfield" enums

jingham added a comment. 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... CHANGES SINCE LAST A

[Lldb-commits] [PATCH] D67520: Add pretty printing of Clang "bitfield" enums

jingham added a comment. Also, since in C++ you can't do: enum bitfield ac = A | C; it's fairly common practice to pass or'ed elements from the enum as an int of some sort. This feature would be really handy in that case, but the way you would get at it is not through frame var (since you can

[Lldb-commits] [PATCH] D67472: [Target] Move InferiorCall to Process

jingham added a comment. In D67472#1668505 , @clayborg wrote: > lgtm. Jim? I already commented above. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67472/new/ https://reviews.llvm.org/D67472 __

[Lldb-commits] [PATCH] D67520: Add pretty printing of Clang "bitfield" enums

jingham added a comment. The code looks fine to me. Comment at: source/Symbol/ClangASTContext.cpp:9502 +++num_enumerators; +if (val == enum_svalue) { + s->PutCString(enumerator->getNameAsString()); Can you put a comment here like: // TADA - we fou

[Lldb-commits] [PATCH] D67523: [Reproducer] Move GDB packet struct into the reproducer. (NFC)

jingham added a comment. 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? Repository: r

[Lldb-commits] [PATCH] D67523: [Reproducer] Move GDB packet struct into the reproducer. (NFC)

jingham added a comment. There's already Utils/StreamGDBRemote.cpp... It might be natural to stick the class in there? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67523/new/ https://reviews.llvm.org/D67523 ___

[Lldb-commits] [PATCH] D67472: [Target] Move InferiorCall to Process

jingham added a comment. I don't think CallNoArgNoReturnFunc is the right name. This routine calls a function that takes no arguments but returns a void *. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67472/new/ https://reviews.llvm.org/D67472

[Lldb-commits] [PATCH] D67472: [Target] Move InferiorCall to Process

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Excellent! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67472/new/ https://reviews.llvm.org/D67472 ___

[Lldb-commits] [PATCH] D67538: [Reproducer] Include the this pointer in the API log.

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67538/new/ https://reviews.llvm.org/D67538 ___ lldb-commi

[Lldb-commits] [PATCH] D67523: [Reproducer] Move GDB Remote Packet into Utility. (NFC)

jingham accepted this revision. jingham added a comment. Looks right. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67523/new/ https://reviews.llvm.org/D67523 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/

[Lldb-commits] [PATCH] D67474: [Reproducer] Add `reproducer dump` command

jingham added a comment. The help for -f should say what happens if this option isn't provided. Otherwise this looks fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67474/new/ https://reviews.llvm.org/D67474 ___ lldb-commits mailing l

[Lldb-commits] [PATCH] D67474: [Reproducer] Add `reproducer dump` command

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Approved with that addition. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67474/new/ https://reviews.llvm.org/D67474 ___ lldb-commits

[Lldb-commits] [PATCH] D67685: [ScriptInterpreter] Make sure LLDB's global variables are only available in interactive mode.

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This looks right. EnterSession always initializes lldb.debugger, even when it isn't going to set the other globals. That's because one debugger owns one ScriptInterpreter and lldb.debugger

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. 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. I don't think anybody woul

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

jingham added a comment. First off, note that this becomes complicated because this patch tied the decision of whether a given stop reason should warrant returning control to the user in lldb's "batch mode", and whether the signal should be passed back to the target. That isn't a necessary bin

[Lldb-commits] [PATCH] D67776: Use UnixSignals::ShouldSuppress to filter out signals a process expects

jingham added a comment. Pavel's example shows why these two choices might be better separated. If you are using __builtin_trap to implement a stop when in the debugger of some sort, it seems likely you would want to suppress the signal, but it also seems likely that when you are running in b

[Lldb-commits] [PATCH] D67776: Don't stop execution in batch mode when process stops with SIGINT or SIGSTOP

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM. Thanks for sticking with this. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67776/new/ https://reviews.llvm.org/D67776 _

[Lldb-commits] [PATCH] D61000: Kill modify-python-lldb.py

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Since this just changes how we fix up comments for the generated functions in lldb.py, I can't see any harm in removing it. It is clearly showing you a C-view of the function, so char -> st

[Lldb-commits] [PATCH] D61044: Fix infinite recursion when calling C++ template functions

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This seems like an okay workaround. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61044/new/ https://reviews.llvm.org/D61044 ___ lldb-

[Lldb-commits] [PATCH] D61090: [SBHostOS} Remove getting the python script interpreter path

jingham requested changes to this revision. jingham added a comment. I agree with Greg, I really don't want to lose the "lldb -P" functionality. That's quite handy. Seems to me that if lldb wants to provide a scripting module for some scripting language, that language would need to know where

[Lldb-commits] [PATCH] D61090: [SBHostOS} Remove getting the python script interpreter path

jingham added a comment. BTW, congrats on getting rid of all the LLDB_DISABLE_PYTHON uses. That was a goodly chunk of work and removed an obvious wart. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61090/new/ https://reviews.llvm.org/D61090 ___

[Lldb-commits] [PATCH] D61090: [SBHostOS} Remove getting the python script interpreter path

jingham added a comment. It seems weird to be getting the ScriptInterpreter's ScriptModuleDirectory from the CommandInterpreter. I would have expected: m_debugger.GetScriptInterpreter().GetScriptModuleDirectory() I don't think there's any reason to have the Command Interpreter know which Scri

[Lldb-commits] [PATCH] D61090: [SBHostOS} Remove getting the python script interpreter path

jingham added a comment. Yeah, I must have not been paying attention at some point in the past. OTOH, I don't know if you can change the access to the ScriptInterpreter in the SB API's, since that seems likely to break people's scripts. Maybe it's better for this patch to add the GetScriptModu

[Lldb-commits] [PATCH] D61090: [SBHostOS} Remove getting the python script interpreter path

jingham added a comment. I'm a bit confused. If we get to the world where we can support multiple script interpreters, I would expect to add a static method on ScriptInterpreter that enumerates the available interpreters. I would not expect that to be on SBHostOS since this is not a feature o

[Lldb-commits] [PATCH] D61090: [SBHostOS} Remove getting the python script interpreter path

jingham added a comment. I really thought there was one at the SB layer, because in terms of design that is what makes sense. I guess we never really needed it until now, so we didn't add it. Once there's more than one hard-coded script interpreter, we will need to add some way to select & di

[Lldb-commits] [PATCH] D61172: [ScriptInterpreter] Pass the debugger instead of the command interpreter

jingham added a comment. Looks good to me too. The fact that you were mostly eliminating redirections from CommandInterpreter -> Debugger is good evidence this was wrongly structured initially. I'm a little curious that we do: m_debugger.GetCommandInterpreter().GetPythonCommandsFromIOHandle

[Lldb-commits] [PATCH] D61172: [ScriptInterpreter] Pass the debugger instead of the command interpreter

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Oops, forgot to do the action part of this... Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61172/new/ https://reviews.llvm.org/D61172 _

[Lldb-commits] [PATCH] D61211: [ScriptInterpreter] Move ownership into debugger (NFC)

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That looks nicer! Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61211/new/ https://reviews.llvm.org/D61211 _

[Lldb-commits] [PATCH] D61451: Hide runtime support values such as clang's __vla_expr from frame variable

jingham added a comment. Could you add a comment (probably best in LanguageRuntime.h) saying what a RuntimeSupportValue is? Also, so far as I can tell, this patch works because we use the CPP language runtime for C++ methods is an ObjC++ file, and the ObjCLanguageRuntime for ObjC methods in di

[Lldb-commits] [PATCH] D61292: Include inlined functions when figuring out a contiguous address range

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This makes sense to me. Pinning the extension to the call site prevented all the cases I could think of where this might go wrong. I had a bunch of small comments inline. =

[Lldb-commits] [PATCH] D61292: Include inlined functions when figuring out a contiguous address range

jingham added inline comments. Comment at: lldb/include/lldb/Symbol/Declaration.h:113 + /// Compares the specification from \a rhs. If the file specifications are + /// equal, then continue to compare the line. + /// clayborg wrote: > How about just returning

[Lldb-commits] [PATCH] D61451: Hide runtime support values such as clang's __vla_expr from frame variable

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

[Lldb-commits] [PATCH] D61438: [ASTImporter] Use llvm::Expected and Error in the importer API

jingham added a comment. IIUC, when Expected returns fails, it returns an Error object that might have information about what went wrong. Would it be possible to include the contents of that error n the log message? We often get "I can't run an expression in a really complex proprietary code

[Lldb-commits] [PATCH] D61469: [Alias] Add 're' alias for register

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61469/new/ https://reviews.llvm.org/D61469 ___ lldb-commi

[Lldb-commits] [PATCH] D61292: Include inlined functions when figuring out a contiguous address range

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM, you should probably wait on Pavel's okay to the testing framework changes since that's more his baby... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[Lldb-commits] [PATCH] D61602: Handle function parameters of RunCommandInterpreter (script bridge)

jingham added a comment. LGTM, thanks for doing this and for the test! Can I ask you to write a doc string showing how to call this and what it returns in Python. It's not entirely obvious. There are %feature("docstring" examples around in that file that you can copy from. Repository: rL

[Lldb-commits] [PATCH] D61483: [www] list command: lldb run

jingham added a comment. The www was the original version, which Jonas replaced with with the rst version. We were waiting to get rid of the www version till we were sure we could iron out all the wrinkles of generating the docs on lldb.llvm.org from the rst version. That's close, though we s

[Lldb-commits] [PATCH] D61578: [Driver] Add command line option to allow loading local lldbinit file

jingham added a comment. In D61578#1493452 , @labath wrote: > In D61578#1492422 , @JDevlieghere > wrote: > > > In D61578#1492086 , @clayborg > > wrote: > > > > > Just wante

[Lldb-commits] [PATCH] D61578: [Driver] Add command line option to allow loading local lldbinit file

jingham added a comment. In D61578#1493875 , @labath wrote: > In D61578#1493863 , @jingham wrote: > > > The dotest.py tests all disable reading the global .lldbinit file. Do the > > lit tests not do that as well?

[Lldb-commits] [PATCH] D61579: Propagate command interpreter errors from lldlbinit

jingham added a comment. I wonder if it would make this a little more convenient if we added a SetNoisy as the opposite of SetSilent? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61579/new/ https://reviews.llvm.org/D61579 __

[Lldb-commits] [PATCH] D61579: Propagate command interpreter errors from lldlbinit

jingham added inline comments. Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2472 + flags |= eHandleCommandFlagPrintErrors; +} else if (m_command_source_flags.back() & eHandleCommandFlagPrintErrors) { + flags |= eHandleCommandFlagPrintErrors; --

[Lldb-commits] [PATCH] D61746: [Breakpoint] Make breakpoint language agnostic

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That looks fine, thanks for untangling this. I do like Jonas' suggestion better. GetVariantMethodNames sounds like "VariantMethods" are a specific thing and you are trying to find their nam

[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. I would rather not clutter up the lldb command driver's options with gdb command flags. That seems like it will make lldb harder to figure out and reduce our freedom to choose rea

[Lldb-commits] [PATCH] D58678: Improve step over performance by not stopping at branches that are function calls and stepping into and them out of each one

jingham added a comment. The one outstanding bit of work here is that this change requires that the MSInst "IsCall" function has to mean "will return to the next instruction after call" or we might lose control of the program. It seems obvious that that SHOULD be what it means, but we need to

[Lldb-commits] [PATCH] D61776: [Target] Generalize some behavior in Thread

jingham added a comment. If you really are going to support many languages you need to figure out how to tell folks what really happened with more specificity. For instance, you can use C++ to throw anything, so you could throw an ObjC object from a C++ exception throw. So you need to disting

[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

jingham added a comment. I really don't want to pollute the lldb driver options with gdb equivalents (or really any duplicate spellings of already present functionality). For the lldb command line, I want us to have the freedom to do the best job of making the lldb options consistent and easy

[Lldb-commits] [PATCH] D61776: [Target] Generalize some behavior in Thread

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. With appropriate comments, this seems fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61776/new/ https://reviews.llvm.org/D61776 _

[Lldb-commits] [PATCH] D61737: [lldb] add -ex CLI option as alias to --one-line

jingham added a comment. In D61737#1498329 , @jankratochvil wrote: > In D61737#1498297 , @jingham wrote: > > > If you have simple gdb startup commands then translating them into the lldb > > way should be straight

[Lldb-commits] [PATCH] D61602: Handle function parameters of RunCommandInterpreter (script bridge)

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Excellent, thanks! Can you check this in or do you need me to? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61602/new/ https://reviews.llvm.org/D61602

[Lldb-commits] [PATCH] D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough

jingham added a comment. There is a TypeSystemEnumerateSupportedLanguages that we use so that we don't have to enumerate over all the language in the languages enums. After all the plugin manager knows which languages we have type systems for. If you're going to be doing a lot of this general

[Lldb-commits] [PATCH] D61602: Handle function parameters of RunCommandInterpreter (script bridge)

This revision was automatically updated to reflect the committed changes. Closed by commit rLLDB360730: Make SBDebugger.RunCommandInterpreter callable from Python. (authored by jingham, committed by ). Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61602/new/ htt

[Lldb-commits] [PATCH] D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough

jingham added a comment. Getting it from the Process's m_language_runtimes is probably fine. On reflection, I can't think of a reason why you would want to iterate over all the available LanguageRuntimes, including the ones that hadn't been recognized in this Process yet. It's fine to do that

[Lldb-commits] [PATCH] D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough

jingham added a comment. In D61921#1502502 , @labath wrote: > If this iteration is going to be used a lot, I'd recommend taking a bit of > time to implement an iterator abstraction over the language runtimes. It > takes a bit longer to set up, but I hope

[Lldb-commits] [PATCH] D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough

jingham added a comment. In D61921#1502352 , @xiaobai wrote: > In D61921#1502338 , @jingham wrote: > > > Getting it from the Process's m_language_runtimes is probably fine. On > > reflection, I can't think of a re

[Lldb-commits] [PATCH] D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough

jingham added a comment. In D61921#1503331 , @xiaobai wrote: > In D61921#1503282 , @jingham wrote: > > > In D61921#1502502 , @labath wrote: > > > > > If this iteration is goi

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

jingham added a comment. Why are we calling this "widehistory" because we couldn't write into the .lldb directory? I know that's the way it was but it makes no sense. I guess it would make sense to call it widehistory if edit line was in wchar mode, that way you wouldn't ask a non wchar edit

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

jingham added a comment. I don't think that's quite right. I think the behavior should be: 1. Try to make a .lldb directory a) If you can, then i) if wchar support ~/.lldb/.lldb-widehistory ii) else ~/.lldb/lldb-history b) else i) if wchar support ~/.lldb-widehistory ii) else ~/.lldb-history

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

jingham added a comment. Most other programs write their history files in ~. So we are being a little odd in offering to put them in ~/.lldb, though I agree that is convenient. But if putting files in ~/.lldb ticked somebody off enough that they made a .lldb directory that was read only, your

[Lldb-commits] [PATCH] D62216: [EditLine] Rewrite GetHistoryFilePath

jingham added a comment. Then put in a comment saying something like "LLDB ONLY stores history files in .lldb and if you don't like that..." If you are instituting a policy which is not common you should at least document it... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62216/new/

[Lldb-commits] [PATCH] D62472: [CMake] LLDB.framework tools handling

jingham added a comment. The typical trick for doing argument substitution before debugging was roughly to debug: exec /bin/sh executable arg1 arg2 ... then follow through the shell to the executable. That failed when SIP made it impossible to debug any of the shells, so we had to come up wit

[Lldb-commits] [PATCH] D62562: [Target] Introduce Process::GetLanguageRuntimes

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

[Lldb-commits] [PATCH] D61921: [Target] Generalize language-specific behavior in ThreadPlanStepThrough

jingham accepted this revision. jingham added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61921/new/ https://reviews.llvm.org/D61921 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/

[Lldb-commits] [PATCH] D62702: [ABI] Fix SystemV ABI to handle nested aggregate type returned in register

jingham added a comment. There should be a test to go along with this. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62702/new/ https://reviews.llvm.org/D62702 ___ lldb-commits mailing list lldb-commits@lists.llvm.

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. This seems like carrying purity a little too far. You haven't removed the fact that the using code is explicitly dialing up the C++ language runtime, you've just made the call-sit

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

jingham added a comment. In D62755#1525919 , @xiaobai wrote: > In D62755#1525890 , @aprantl wrote: > > > I don't yet see the connection between those goals and this patch, but I > > might be missing something. Woul

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This deals with my objections so I'm marking as accepted because that's the only way I can see to unblock. Seems like you were still discussing return types, and I don't want to preempt tha

[Lldb-commits] [PATCH] D62755: [Target] Remove Process::GetCPPLanguageRuntime

jingham added a comment. Fine by me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62755/new/ https://reviews.llvm.org/D62755 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-com

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::GetCompilerTypeFromPersistentDecl

jingham added a comment. I have no problem with the change in general. However, you've introduced the possibility of name collision between convenience type definition in various languages. So it would be good to run through the persistent DECL's for ALL the supported languages and report col

[Lldb-commits] [PATCH] D62795: [Target] Move ObjCLanguageRuntime::LookupRuntimeSymbol into LanguageRuntime

jingham added a comment. This looks fine to me. For context, ObjC has symbols that point into the runtime that tell you things like the current offsets of the members of an ObjC class. In debug builds the symbols are present, but the runtime doesn't depend on the symbols per se, since it read

[Lldb-commits] [PATCH] D62797: [Expression] Add PersistentExpressionState::GetCompilerTypeFromPersistentDecl

jingham added a comment. Pretty much the only actual effect of this patch (besides its changes to the dependency graph) are introducing the possibility for ambiguity here. It seems more logical to do it all as one patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62797/new/ https:

[Lldb-commits] [PATCH] D62887: Update the thread list before setting stop reasons with an OS plugin

jingham created this revision. jingham added a reviewer: clayborg. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. When talking to a gdb-remote server that didn't support the newer thread info packets, we noticed that lldb would hit a breakpoint in target, but then continu

[Lldb-commits] [PATCH] D63181: [Target] Decouple ObjCLanguageRuntime from LanguageRuntime

jingham added a comment. In D63181#1539292 , @xiaobai wrote: > In D63181#1539291 , @JDevlieghere > wrote: > > > Have you considered making just `AddExceptionPrecondition` virtual? > > Wouldn't that solve the probl

[Lldb-commits] [PATCH] D63181: [Target] Decouple ObjCLanguageRuntime from LanguageRuntime

jingham added a comment. This looks fine to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63181/new/ https://reviews.llvm.org/D63181 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listin

[Lldb-commits] [PATCH] D63363: [Signals] Create a plugin directory just for signals

jingham added a comment. In D63363#1545427 , @labath wrote: > Although this is technically correct and pretty consistent with our other > "plugins", I can't help but feel that it's incredibly wasteful. Each of the > XXXSignals.cpp files is less than a 10

[Lldb-commits] [PATCH] D63622: [Target] Hoist LanguageRuntime::GetDeclVendor

jingham added a comment. This change makes it clear that SBTarget::FindFirstType should take a language, but that is orthogonal to this change. Comment at: source/API/SBTarget.cpp:1854-1859 + if (vendor->FindDecls(const_typename, /*append*/ true, +

[Lldb-commits] [PATCH] D63622: [Target] Hoist LanguageRuntime::GetDeclVendor

jingham added inline comments. Comment at: source/API/SBTarget.cpp:1854-1859 + if (vendor->FindDecls(const_typename, /*append*/ true, +/*max_matches*/ 1, decls) > 0) { +if (CompilerType type = +ClangASTConte

[Lldb-commits] [PATCH] D63622: [Target] Hoist LanguageRuntime::GetDeclVendor

jingham added inline comments. Comment at: include/lldb/Target/LanguageRuntime.h:137 + virtual DeclVendor *GetDeclVendor() { return nullptr; } + compnerd wrote: > Can this not be `const`? Seems like retrieving the vendor should not mutate > the runtime. The

[Lldb-commits] [PATCH] D63622: [Target] Hoist LanguageRuntime::GetDeclVendor

jingham added a subscriber: labath. jingham added a comment. In D63622#1553121 , @labath wrote: > Seems like a reasonable thing to do, but I don't really know what this code > does... The runtime DeclVendor gives runtimes a way to produce type informati

[Lldb-commits] [PATCH] D63622: [Target] Hoist LanguageRuntime::GetDeclVendor

jingham added inline comments. Comment at: source/API/SBTarget.cpp:1854-1859 + if (vendor->FindDecls(const_typename, /*append*/ true, +/*max_matches*/ 1, decls) > 0) { +if (CompilerType type = +ClangASTConte

[Lldb-commits] [PATCH] D63622: [Target] Hoist LanguageRuntime::GetDeclVendor

jingham added inline comments. Comment at: source/API/SBTarget.cpp:1854-1859 + if (vendor->FindDecls(const_typename, /*append*/ true, +/*max_matches*/ 1, decls) > 0) { +if (CompilerType type = +ClangASTConte

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham added inline comments. Comment at: source/Core/ValueObject.cpp:1719 +// artificial. return GetVariable() && GetVariable()->IsArtificial(); } clayborg wrote: > Things brings the questions: do we really need to filter these variables? I > would

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham requested changes to this revision. jingham added inline comments. This revision now requires changes to proceed. Comment at: source/Core/ValueObject.cpp:1706-1708 + for (auto *runtime : process->GetLanguageRuntimes()) { +if (runtime->IsWhitelistedRuntimeValue(GetNam

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham added a comment. Shouldn't ValueObjectVariables figure out their runtime language from their defining frame, not their CompilerType? For a ValueObject you get from an expression, you probably can't do that. But we're always talking about hiding locals or args here - i.e. they are all

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham added a comment. In D63240#1559931 , @xiaobai wrote: > In D63240#1559913 , @jingham wrote: > > > Shouldn't ValueObjectVariables figure out their runtime language from their > > defining frame, not their Com

[Lldb-commits] [PATCH] D63240: [Core] Generalize ValueObject::IsRuntimeSupportValue

jingham added a comment. In D63240#1560038 , @xiaobai wrote: > In D63240#1559994 , @jingham wrote: > > > To be more precise, "frame" is the wrong word to use, Variables have > > "scopes"... All Variables have a sc

<    11   12   13   14   15   16   17   18   19   >