[Lldb-commits] [PATCH] D54617: [wip][Reproducers] Add file provider

2018-11-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. In https://reviews.llvm.org/D54617#1302366, @labath wrote: > I am confused by differing scopes of various objects that are interacting > here. It seems you are implementing capture/replay as something that can be > flicked on/off at any point during a debug

[Lldb-commits] [PATCH] D54544: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

2018-11-18 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a subscriber: lanza. zturner added a comment. I had to revert this because it breaks many tests on Windows (found by bisecting). It was reverted in r347174. You can reproduce one of the failures by building with this patch applied, then doing python bin/llvm-lit.py -sv

Re: [Lldb-commits] [PATCH] D54544: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

2018-11-18 Thread Zachary Turner via lldb-commits
I had to revert this because it breaks many tests on Windows (found by bisecting). It was reverted in r347174. You can reproduce one of the failures by building with this patch applied, then doing python bin/llvm-lit.py -sv ~/src/lldb/lit/SymbolFile/PDB. 2 of the tests should fail and you see

[Lldb-commits] [lldb] r347174 - Revert "Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD"

2018-11-18 Thread Zachary Turner via lldb-commits
Author: zturner Date: Sun Nov 18 12:48:25 2018 New Revision: 347174 URL: http://llvm.org/viewvc/llvm-project?rev=347174=rev Log: Revert "Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD" This breaks many tests on Windows, which now all fail with an error such as "Unable to

[Lldb-commits] [PATCH] D54680: Don't use lldb -O in lit tests

2018-11-18 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment. In https://reviews.llvm.org/D54680#1302408, @davide wrote: > Is there a way of fixing this that doesn't require scattering the test > between two files? Unless we create a utility that extracts lines based on prefixes and outputs them to a temporary file, I don't

[Lldb-commits] [PATCH] D54680: Don't use lldb -O in lit tests

2018-11-18 Thread Frederic Riss via Phabricator via lldb-commits
friss added a comment. I do agree it's slightly easier to read this way. I was conflicted while writing the tests between readability and conciseness. I think this is a good compromise. I do prefer to have the check lines with the commands rather than with the source code though. The output

[Lldb-commits] [PATCH] D54682: [Driver] Extract option parsing and option processing.

2018-11-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere created this revision. JDevlieghere added reviewers: clayborg, labath, zturner, jingham. JDevlieghere added a project: LLDB. In order to deal consistently with global state in LLDB, the reproducer feature affects LLDB's initialization. For example, when replaying, the FileSystem

Re: [Lldb-commits] [PATCH] D54682: [Driver] Extract option parsing and option processing.

2018-11-18 Thread Zachary Turner via lldb-commits
I’ve often thought we should convert LLDB’s command line parsing code over to use either cl::opt or lib/Option. This would also solve the problem you describe here at the same time. Do you think it’s worth trying to do this? On Sun, Nov 18, 2018 at 7:17 PM Jonas Devlieghere via Phabricator <

[Lldb-commits] [PATCH] D54682: [Driver] Extract option parsing and option processing.

2018-11-18 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a subscriber: JDevlieghere. zturner added a comment. I’ve often thought we should convert LLDB’s command line parsing code over to use either cl::opt or lib/Option. This would also solve the problem you describe here at the same time. Do you think it’s worth trying to do this?

[Lldb-commits] [PATCH] D54682: [Driver] Extract option parsing and option processing.

2018-11-18 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added a comment. In https://reviews.llvm.org/D54682#1302436, @zturner wrote: > I’ve often thought we should convert LLDB’s command line parsing code over > to use either cl::opt or lib/Option. This would also solve the problem you > describe here at the same time. > > Do you think

[Lldb-commits] [PATCH] D54680: Don't use lldb -O in lit tests

2018-11-18 Thread Davide Italiano via Phabricator via lldb-commits
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. I don't have any other great ideas either and I'd say the proposed alternative is not worth the complexity. https://reviews.llvm.org/D54680 ___

Re: [Lldb-commits] [PATCH] D54682: [Driver] Extract option parsing and option processing.

2018-11-18 Thread Zachary Turner via lldb-commits
Lib option definitely does, as its entire purpose is to be powerful and flexible enough to mimic arbitrary command line tools. cl::opt is a little less powerful but it still preserves order among multiple options with the same flag, just not multiple options with different flags. I don’t have a

[Lldb-commits] [PATCH] D54682: [Driver] Extract option parsing and option processing.

2018-11-18 Thread Zachary Turner via Phabricator via lldb-commits
zturner added a comment. Lib option definitely does, as its entire purpose is to be powerful and flexible enough to mimic arbitrary command line tools. cl::opt is a little less powerful but it still preserves order among multiple options with the same flag, just not multiple options with

[Lldb-commits] [PATCH] D54680: Don't use lldb -O in lit tests

2018-11-18 Thread Zachary Turner via Phabricator via lldb-commits
zturner created this revision. zturner added a reviewer: friss. Because of different shell quoting rules, and the fact that LLDB commands often contain spaces, -O is not portable for writing command lines. Instead, we should use explicit lldbinit files. https://reviews.llvm.org/D54680

[Lldb-commits] [PATCH] D54680: Don't use lldb -O in lit tests

2018-11-18 Thread Davide Italiano via Phabricator via lldb-commits
davide requested changes to this revision. davide added a comment. This revision now requires changes to proceed. Is there a way of fixing this that doesn't require scattering the test between two files? https://reviews.llvm.org/D54680 ___

Re: [Lldb-commits] [lldb] r347109 - Rewrite stop-hook tests as a couple of FileCheck tests

2018-11-18 Thread Zachary Turner via lldb-commits
Different shells have different quoting rules, so unfortunately something like this: # RUN: %lldb -b -s %s -O 'target create %t' -O 'target stop-hook add -n b -o "expr ptr"' is non-portable. For example, on Windows all single quotes are converted to double quotes before running, so this is

[Lldb-commits] [PATCH] D54617: [wip][Reproducers] Add file provider

2018-11-18 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I am confused by differing scopes of various objects that are interacting here. It seems you are implementing capture/replay as something that can be flicked on/off at any point during a debug session (`Debugger` lifetime). However, you are modifying global state (the