Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

I'll add tests if it looks like it'll be accepted, but based on the initial response, that doesn't seem likely. However, it was a good exercise and addressed the issues raised. thanks again for all the feedback... don On Mon, Oct 30, 2017 at 9:44 PM, Zachary Turner wrote: > Asking again, but w

Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

Asking again, but why can’t this be done in th the script for clangdiag? For example, there’s no tests for any of this in this patch. And it seems likely that it will be rarely used anyway. So I’m still not convinced the option-pollution and increased long term code maintenance burden of this under

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

hintonda updated this revision to Diff 120933. hintonda added a comment. - Remove prefix and add options. https://reviews.llvm.org/D39436 Files: include/lldb/Utility/FileSpec.h source/Commands/CommandObjectBreakpoint.cpp source/Utility/FileSpec.cpp Index: source/Utility/FileSpec.cpp

Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

On Mon, Oct 30, 2017 at 5:47 PM Jim Ingham via Phabricator < revi...@reviews.llvm.org> wrote: > jingham added a comment. > > In https://reviews.llvm.org/D39436#911305, @zturner wrote: > > > In https://reviews.llvm.org/D39436#911302, @hintonda wrote: > > > > > In https://reviews.llvm.org/D39436#911

Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

> On Oct 30, 2017, at 5:35 PM, Zachary Turner wrote: > > > On Mon, Oct 30, 2017 at 5:32 PM Jim Ingham via Phabricator > mailto:revi...@reviews.llvm.org>> wrote: > jingham added a comment. > > In https://reviews.llvm.org/D39436#911304 > , @hintonda wro

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

jingham added a comment. In https://reviews.llvm.org/D39436#911305, @zturner wrote: > In https://reviews.llvm.org/D39436#911302, @hintonda wrote: > > > In https://reviews.llvm.org/D39436#911264, @jingham wrote: > > > > > Zachary's suggestion is better than adding regex patterns to FileSpec, > >

Re: [Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

On Mon, Oct 30, 2017 at 5:32 PM Jim Ingham via Phabricator < revi...@reviews.llvm.org> wrote: > jingham added a comment. > > In https://reviews.llvm.org/D39436#911304, @hintonda wrote: > > > In https://reviews.llvm.org/D39436#911274, @jingham wrote: > > > > > BTW, to Z's comment: you can't really

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoints.

hintonda created this revision. Add regex support to file and module names when setting breakpoints. This solution is enabled via a "regex:" prefix, and doesn't require any other api changes. In this example, setting breakpoints for a particular function was twice as fast using "regex:.*/clang/.*

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

jingham added a comment. In https://reviews.llvm.org/D39436#911304, @hintonda wrote: > In https://reviews.llvm.org/D39436#911274, @jingham wrote: > > > BTW, to Z's comment: you can't really resolve the regex pattern when you > > make the breakpoint. What if another library gets loaded later on,

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

hintonda added a comment. In https://reviews.llvm.org/D39436#911296, @jingham wrote: > In https://reviews.llvm.org/D39436#911293, @hintonda wrote: > > > In https://reviews.llvm.org/D39436#911260, @jingham wrote: > > > > > I don't like this change. > > > > > > First off, the whole point of having

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

hintonda added a comment. In https://reviews.llvm.org/D39436#911274, @jingham wrote: > BTW, to Z's comment: you can't really resolve the regex pattern when you make > the breakpoint. What if another library gets loaded later on, which has > source files that match the source file pattern the u

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

zturner added a comment. In https://reviews.llvm.org/D39436#911302, @hintonda wrote: > In https://reviews.llvm.org/D39436#911264, @jingham wrote: > > > Zachary's suggestion is better than adding regex patterns to FileSpec, but > > I still don't like the idea of encoding option types in the optio

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

hintonda added a comment. In https://reviews.llvm.org/D39436#911264, @jingham wrote: > Zachary's suggestion is better than adding regex patterns to FileSpec, but I > still don't like the idea of encoding option types in the option values. Are you talking about fnmatch? Is that portable? If n

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

jingham added a comment. I do think "all inlines of line 7 of my_cool_templates.h in some_file.cpp" is potentially useful. So adding a "source-pattern" and using it as a limiter for a search in much the same way --shlib is used seems generally useful to me. So it seems worthwhile adding this

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

jingham added a comment. In https://reviews.llvm.org/D39436#911293, @hintonda wrote: > In https://reviews.llvm.org/D39436#911260, @jingham wrote: > > > I don't like this change. > > > > First off, the whole point of having options in the commands is so that we > > don't have to have magic encodi

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

hintonda added a comment. In https://reviews.llvm.org/D39436#911260, @jingham wrote: > I don't like this change. > > First off, the whole point of having options in the commands is so that we > don't have to have magic encodings in the values. > > We also don't have FileSpec's that resolve to mu

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

hintonda added a comment. In https://reviews.llvm.org/D39436#911263, @zturner wrote: > In https://reviews.llvm.org/D39436#911254, @hintonda wrote: > > > In https://reviews.llvm.org/D39436#911237, @zturner wrote: > > > > > Hmm, weird. Maybe it already is? Even though I didn't see it on the > >

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

zturner added a comment. In https://reviews.llvm.org/D39436#911264, @jingham wrote: > Zachary's suggestion is better than adding regex patterns to FileSpec, but I > still don't like the idea of encoding option types in the option values. > > Moreover, this doesn't really apply to the main use of

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

jingham added a comment. Zachary's suggestion is better than adding regex patterns to FileSpec, but I still don't like the idea of encoding option types in the option values. Moreover, this doesn't really apply to the main use of -f - file & line breakpoints. The only plausible use for a sourc

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

jingham added a comment. BTW, to Z's comment: you can't really resolve the regex pattern when you make the breakpoint. What if another library gets loaded later on, which has source files that match the source file pattern the user entered, and have source code that matches the -p pattern. Th

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

jingham requested changes to this revision. jingham added a comment. This revision now requires changes to proceed. I don't like this change. First off, the whole point of having options in the commands is so that we don't have to have magic encodings in the values. We also don't have FileSpec'

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

zturner added a comment. In https://reviews.llvm.org/D39436#911254, @hintonda wrote: > In https://reviews.llvm.org/D39436#911237, @zturner wrote: > > > Hmm, weird. Maybe it already is? Even though I didn't see it on the > > original email. Anyway, ignore my last suggestion. > > > > On to actu

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

hintonda added a comment. In https://reviews.llvm.org/D39436#911237, @zturner wrote: > Hmm, weird. Maybe it already is? Even though I didn't see it on the > original email. Anyway, ignore my last suggestion. > > On to actual comments: I'm not really crazy about the `regex:` prefix. Seems >

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

hintonda added a comment. In https://reviews.llvm.org/D39436#911233, @zturner wrote: > Would you mind deleting and re-creating this revision with lldb-commits added > as a subscriber? I don't think it's sufficient to just "add" it as a > subscriber after the fact, I think it has to be done as

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

zturner added a comment. Hmm, weird. Maybe it already is? Even though I didn't see it on the original email. Anyway, ignore my last suggestion. On to actual comments: I'm not really crazy about the `regex:` prefix. Seems ugly and non-intuitive. Couldn't we just treat every filename as an a

[Lldb-commits] [PATCH] D39436: Add regex support to file (-f) and module (-s) breakpoint options.

zturner added a comment. Would you mind deleting and re-creating this revision with lldb-commits added as a subscriber? I don't think it's sufficient to just "add" it as a subscriber after the fact, I think it has to be done as part of the initial creation, or for some reason it won't show up

[Lldb-commits] [lldb] r316954 - Remove a stray space.

Author: jingham Date: Mon Oct 30 13:44:45 2017 New Revision: 316954 URL: http://llvm.org/viewvc/llvm-project?rev=316954&view=rev Log: Remove a stray space. Modified: lldb/trunk/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp Modified: lldb/trunk/source/Plugins/Lan

[Lldb-commits] [PATCH] D39429: Use LLVM version string

sas created this revision. macOS builds of LLDB use the bundle version from `tools/driver/lldb-Info.plist`. That file hasn't been updated since the 4.0 release so the version we print provides no value. I also think that even if it were up to date, that number has no meaning and displaying the ver

Re: [Lldb-commits] [PATCH] D39307: Fix global data symbol resolution

> On Oct 26, 2017, at 9:34 AM, Zachary Turner > wrote: > > Note that $ is a valid character in a MSVC-mangled symbol name. So, I don't > think it will work for that reason alone. FWIW, I also don't like the {,,} > syntax very much, but if you read on there's a sim

[Lldb-commits] [PATCH] D35556: Add data formatter for libc++'s forward_list

jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Great, this is much nicer. Comment at: source/Plugins/Language/CPlusPlus/LibCxxList.cpp:141 -namespace lldb_private { -namespace formatters { -class LibcxxStdListSyntheti

[Lldb-commits] [lldb] r316919 - Fix windows build broken in r316915

Author: labath Date: Mon Oct 30 09:21:18 2017 New Revision: 316919 URL: http://llvm.org/viewvc/llvm-project?rev=316919&view=rev Log: Fix windows build broken in r316915 I accidentally left a linux-specific include in generic code. Modified: lldb/trunk/source/Host/common/MainLoop.cpp Modifie

[Lldb-commits] [lldb] r316915 - MainLoop: work around an android libc bug

Author: labath Date: Mon Oct 30 09:00:13 2017 New Revision: 316915 URL: http://llvm.org/viewvc/llvm-project?rev=316915&view=rev Log: MainLoop: work around an android libc bug Versions of android before kitkat implemented pselect non-atomically, which caused flakyness, as we were relying on it ato