[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

2022-05-24 Thread Luboš Luňák via Phabricator via lldb-commits
llunak added a comment. In D122974#3535669 , @dblaikie wrote: >> It doesn't make sense to require a stable hash algorithm for an internal >> cache file. > > It's at least a stronger stability requirement than may be provided before - > like: stable acro

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

2022-05-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments. Comment at: lldb/include/lldb/Target/Target.h:1451 + /// Print all the signals set in this target. + void PrintDummySignals(Stream &strm, Args signals, size_t num_signals); + Was `Args` supposed to be a reference here? If not

[Lldb-commits] [PATCH] D125148: Add an example command that runs to one of a set of breakpoints

2022-05-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments. Comment at: lldb/examples/python/cont_to_bkpt.py:5 +def __init__(self, debugger, unused): +return + Nit: The "Pythonic" way of doing is using `pass` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

2022-05-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. In D126109#3534522 , @hawkinsw wrote: > @DavidSpickett Thanks (again) for the feedback. Once I fix the nit, is the > proper protocol for you to review again? I am sorry to ask but I don't know > the right procedure and don'

[Lldb-commits] [PATCH] D126217: [lldb] Improve TestAppleSimulatorOSType.py error message

2022-05-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG436eaf8d32fa: [lldb] Improve TestAppleSimulatorOSType.py error message (authored by JDevlieghere). Herald added a project: LLDB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[Lldb-commits] [lldb] 436eaf8 - [lldb] Improve TestAppleSimulatorOSType.py error message

2022-05-24 Thread Jonas Devlieghere via lldb-commits
Author: Jonas Devlieghere Date: 2022-05-24T17:17:26-07:00 New Revision: 436eaf8d32fa8d5db6782f4da30d0b5b7c2690e8 URL: https://github.com/llvm/llvm-project/commit/436eaf8d32fa8d5db6782f4da30d0b5b7c2690e8 DIFF: https://github.com/llvm/llvm-project/commit/436eaf8d32fa8d5db6782f4da30d0b5b7c2690e8.d

[Lldb-commits] [lldb] f179f40 - [lldb] Disable modules in Apple-lldb-base

2022-05-24 Thread Jonas Devlieghere via lldb-commits
Author: Jonas Devlieghere Date: 2022-05-24T17:17:14-07:00 New Revision: f179f403c8579c939e09d27c383b6263cf2523aa URL: https://github.com/llvm/llvm-project/commit/f179f403c8579c939e09d27c383b6263cf2523aa DIFF: https://github.com/llvm/llvm-project/commit/f179f403c8579c939e09d27c383b6263cf2523aa.d

[Lldb-commits] [PATCH] D125148: Add an example command that runs to one of a set of breakpoints

2022-05-24 Thread Dave Lee via Phabricator via lldb-commits
kastiglione added a comment. > The patch gets a little hard to read with all the no-longer in the right > places comments I just learned there's a keyboard shortcut (`A`) that hides them all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125148/ne

[Lldb-commits] [PATCH] D125148: Add an example command that runs to one of a set of breakpoints

2022-05-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham marked 7 inline comments as done. jingham added a comment. The patch gets a little hard to read with all the no-longer in the right places comments, but I think I addressed everything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125148/ne

[Lldb-commits] [PATCH] D125148: Add an example command that runs to one of a set of breakpoints

2022-05-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham updated this revision to Diff 431837. jingham added a comment. Added support for breakpoint names as well as ID's. Addressed other review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125148/new/ https://reviews.llvm.org/D125148

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

2022-05-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. In D126259#3535688 , @jingham wrote: > In D126259#3535004 , @clayborg > wrote: > >> In D126259#3534997 <

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

2022-05-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D126259#3535004 , @clayborg wrote: > In D126259#3534997 , @jingham wrote: > >> Don't allow setting signal actions by signal number before you have a >> process. > > I understand > >> W

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

2022-05-24 Thread David Blaikie via Phabricator via lldb-commits
dblaikie added a comment. > It doesn't make sense to require a stable hash algorithm for an internal > cache file. It's at least a stronger stability requirement than may be provided before - like: stable across process boundaries, at least, by the sounds of it? yeah? & then still raises the qu

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

2022-05-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In D126259#3534997 , @jingham wrote: > Don't allow setting signal actions by signal number before you have a process. I understand > We don't know what signal 20 is going to end up being till we have a process, > so allowing t

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

2022-05-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D126259#3534944 , @clayborg wrote: > In D126259#3534919 , @jingham wrote: > >> In D126259#3534729 , @clayborg >> wrote: >> >>> So the "process

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

2022-05-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham updated this revision to Diff 431745. jingham added a comment. Don't allow setting signal actions by signal number before you have a process. We don't know what signal 20 is going to end up being till we have a process, so allowing this by number doesn't make sense. Repository: rG LL

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

2022-05-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. In D126259#3534919 , @jingham wrote: > In D126259#3534729 , @clayborg > wrote: > >> So the "process handle" command allows us to set signals by signal number as >> well. Does this patch

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

2022-05-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham marked 3 inline comments as done. jingham added a comment. In D126259#3534729 , @clayborg wrote: > So the "process handle" command allows us to set signals by signal number as > well. Does this patch support this? It seems like it wouldn't be too

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

2022-05-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment. So the "process handle" command allows us to set signals by signal number as well. Does this patch support this? It seems like it wouldn't be too hard to do if we wanted to handle this. Lemme know what you think, other than that LGTM. Comment at: lld

[Lldb-commits] [PATCH] D126259: Add the ability to specify signal actions (pass, stop or notify) for a signal before a process is created

2022-05-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham marked 3 inline comments as done. jingham added a comment. This patch treats the Signal structure held in the target differently from UnixSignals::Signal. The latter always has to have values for all three actions and needs to have a signal number. The former doesn't actually claim to

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

2022-05-24 Thread Will Hawkins via Phabricator via lldb-commits
hawkinsw added a comment. @DavidSpickett Thanks (again) for the feedback. Once I fix the nit, is the proper protocol for you to review again? I am sorry to ask but I don't know the right procedure and don't want to do the wrong thing! Comment at: lldb/test/API/functionalitie

[Lldb-commits] [PATCH] D126109: [lldb] Fix broken bad-address-breakpoint test

2022-05-24 Thread David Spickett via Phabricator via lldb-commits
DavidSpickett accepted this revision. DavidSpickett added a comment. This revision is now accepted and ready to land. Much clearer now, thanks. LGTM with the one nit fixed. Comment at: lldb/test/API/functionalities/breakpoint/address_breakpoints/TestBadAddressBreakpoints.py:42