* patch for pr9067 @ 2010-02-02 22:12 Chris Moller 2010-02-02 23:28 ` Tom Tromey [not found] ` <m3bpg7yznc.fsf@fleche.redhat.com> 0 siblings, 2 replies; 19+ messages in thread From: Chris Moller @ 2010-02-02 22:12 UTC (permalink / raw) To: Tom Tromey; +Cc: Project Archer Tom I just sent $subject to gdb-patches. Turns out it was simultaneously more complex and simpler than we both thought: You can't just blindly inhibit multiple printings of static members. Sometimes it's legitimate, like when they legitimately appear multiple times, possibly on different paths through the tree. The simple side is that after I'd finished writing a tree-path-aware fix for the bug, while I was testing it, I noticed that someone had already stuck in an attempted fix, using obstacks, but had screwed up their use. Not that I'm a big fan of obstacks, but on the least-tinkering-possible principle, I scrapped my new stuff and fixed the obstacks. Chris ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: patch for pr9067 2010-02-02 22:12 patch for pr9067 Chris Moller @ 2010-02-02 23:28 ` Tom Tromey 2010-02-03 2:25 ` Chris Moller [not found] ` <m3bpg7yznc.fsf@fleche.redhat.com> 1 sibling, 1 reply; 19+ messages in thread From: Tom Tromey @ 2010-02-02 23:28 UTC (permalink / raw) To: Chris Moller; +Cc: Project Archer >>>>> "Chris" == Chris Moller <cmoller@redhat.com> writes: Chris> I just sent $subject to gdb-patches. Thanks. Chris> Turns out it was simultaneously more complex and simpler than we Chris> both thought: You can't just blindly inhibit multiple printings Chris> of static members. Sometimes it's legitimate, like when they Chris> legitimately appear multiple times, possibly on different paths Chris> through the tree. I didn't see this in the test case you sent. Could you add it? Chris> Not that I'm a big fan of obstacks, but on the Chris> least-tinkering-possible principle, I scrapped my new stuff and Chris> fixed the obstacks. Yeah, that's the right thing to do. Tom ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: patch for pr9067 2010-02-02 23:28 ` Tom Tromey @ 2010-02-03 2:25 ` Chris Moller 2010-02-03 20:04 ` Tom Tromey 0 siblings, 1 reply; 19+ messages in thread From: Chris Moller @ 2010-02-03 2:25 UTC (permalink / raw) To: archer On 02/02/10 18:28, Tom Tromey wrote: >>>>>> "Chris" == Chris Moller<cmoller@redhat.com> writes: >>>>>> > > Chris> I just sent $subject to gdb-patches. > > Thanks. > > Chris> Turns out it was simultaneously more complex and simpler than we > Chris> both thought: You can't just blindly inhibit multiple printings > Chris> of static members. Sometimes it's legitimate, like when they > Chris> legitimately appear multiple times, possibly on different paths > Chris> through the tree. > > I didn't see this in the test case you sent. > Could you add it? > Sure, I can add it, but there are a handful of existing test cases that already cover that circumstance--I found them the hard way when my simplistic first-pas "fix" kept filtering out legitimate instances. > Chris> Not that I'm a big fan of obstacks, but on the > Chris> least-tinkering-possible principle, I scrapped my new stuff and > Chris> fixed the obstacks. > > Yeah, that's the right thing to do. > > Tom > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: patch for pr9067 2010-02-03 2:25 ` Chris Moller @ 2010-02-03 20:04 ` Tom Tromey 0 siblings, 0 replies; 19+ messages in thread From: Tom Tromey @ 2010-02-03 20:04 UTC (permalink / raw) To: Chris Moller; +Cc: archer Chris> Sure, I can add it, but there are a handful of existing test cases Chris> that already cover that circumstance--I found them the hard way when Chris> my simplistic first-pas "fix" kept filtering out legitimate instances. If it is triggered by the existing test suite, then that is good enough, thanks. Tom ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <m3bpg7yznc.fsf@fleche.redhat.com>]
[parent not found: <4B68DC25.1090808@redhat.com>]
[parent not found: <m3fx5irwx6.fsf@fleche.redhat.com>]
* PR 11067 [not found] ` <m3fx5irwx6.fsf@fleche.redhat.com> @ 2010-02-03 21:52 ` Chris Moller 2010-02-03 22:43 ` Roland McGrath 2010-02-03 22:47 ` Tom Tromey 0 siblings, 2 replies; 19+ messages in thread From: Chris Moller @ 2010-02-03 21:52 UTC (permalink / raw) To: Tom Tromey; +Cc: Eric Bachalo, Project Archer On 02/03/10 13:33, Tom Tromey wrote: > Then how about http://sourceware.org/bugzilla/show_bug.cgi?id=11067 ? > I just happened to run across this the other day; if it doesn't suit, I > can dig through bugzilla and find something else. > The enhancement req reads > When I print a variable, printing the symbolic name for its value is right, but > when I print the symbolic name, gdb should print its numeric value instead (or > in addition). > The simple fix is to always print both the symbolic name and the decimal value: > (gdb) p e > $1 = Val1 (= 56) > (gdb) p Val1 > $2 = Val1 (= 56) > (gdb) That's a one-line change in c-valprint.c. Trying to differentiate between those two cases looks like it would be significantly more difficult. Would the one-line change do? Or does the fix need to do exactly what the request says? Chris > Tom > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-03 21:52 ` PR 11067 Chris Moller @ 2010-02-03 22:43 ` Roland McGrath 2010-02-03 22:48 ` Tom Tromey 2010-02-03 22:47 ` Tom Tromey 1 sibling, 1 reply; 19+ messages in thread From: Roland McGrath @ 2010-02-03 22:43 UTC (permalink / raw) To: Chris Moller; +Cc: Tom Tromey, Eric Bachalo, Project Archer > The simple fix is to always print both the symbolic name and the decimal > value: > > > (gdb) p e > > $1 = Val1 (= 56) > > (gdb) p Val1 > > $2 = Val1 (= 56) > > (gdb) I think it should use a format consistent with p/c, e.g.: (gdb) p 'a' $1 = 97 'a' (gdb) p ENUM1 $2 = (enum frobozz) 1 ENUM1 (gdb) Thanks, Roland ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-03 22:43 ` Roland McGrath @ 2010-02-03 22:48 ` Tom Tromey 2010-02-03 23:14 ` Daniel Jacobowitz 2010-02-03 23:21 ` Chris Moller 0 siblings, 2 replies; 19+ messages in thread From: Tom Tromey @ 2010-02-03 22:48 UTC (permalink / raw) To: Roland McGrath; +Cc: Chris Moller, Eric Bachalo, Project Archer >>>>> "Roland" == Roland McGrath <roland@redhat.com> writes: Roland> I think it should use a format consistent with p/c, e.g.: Roland> (gdb) p 'a' Roland> $1 = 97 'a' Roland> (gdb) p ENUM1 Roland> $2 = (enum frobozz) 1 ENUM1 Works for me. Chris, please do it this way, thanks. Tom ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-03 22:48 ` Tom Tromey @ 2010-02-03 23:14 ` Daniel Jacobowitz 2010-02-03 23:25 ` Roland McGrath 2010-02-03 23:21 ` Chris Moller 1 sibling, 1 reply; 19+ messages in thread From: Daniel Jacobowitz @ 2010-02-03 23:14 UTC (permalink / raw) To: Tom Tromey; +Cc: Roland McGrath, Chris Moller, Eric Bachalo, Project Archer On Wed, Feb 03, 2010 at 03:48:20PM -0700, Tom Tromey wrote: > >>>>> "Roland" == Roland McGrath <roland@redhat.com> writes: > > Roland> I think it should use a format consistent with p/c, e.g.: > Roland> (gdb) p 'a' > Roland> $1 = 97 'a' > Roland> (gdb) p ENUM1 > Roland> $2 = (enum frobozz) 1 ENUM1 > > Works for me. Chris, please do it this way, thanks. FWIW, we've had trouble in the past with the 97 'a' output, because you get something out that you can't paste back in (not a valid expression). I know I did when working on C++ template value parameters, but that's a special case. More importantely, I believe MI frontends had some trouble. Roland's example is unintuitive to me since "1 ENUM1" just makes me think "what is that? it's not C, how are those two things related?". The closest C language version would be "ENUM1 = 1". -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-03 23:14 ` Daniel Jacobowitz @ 2010-02-03 23:25 ` Roland McGrath 2010-02-03 23:30 ` Daniel Jacobowitz 0 siblings, 1 reply; 19+ messages in thread From: Roland McGrath @ 2010-02-03 23:25 UTC (permalink / raw) To: Daniel Jacobowitz; +Cc: Tom Tromey, Chris Moller, Eric Bachalo, Project Archer > Roland's example is unintuitive to me since "1 ENUM1" just makes me > think "what is that? it's not C, how are those two things related?". > The closest C language version would be "ENUM1 = 1". Yeah, I was going for parity and the /c format would not have been my first choice if we didn't already have it. I think: $2 = (enum frobozz) 1 = ENUM1 would be fine too, or maybe reverse the clauses. "(type) digit" is probably what's most likely always to be pasteable anywhere, in case it's into a context where you don't have the same scope for the enum constant (in C++). Thanks, Roland ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-03 23:25 ` Roland McGrath @ 2010-02-03 23:30 ` Daniel Jacobowitz 2010-02-03 23:34 ` Chris Moller 0 siblings, 1 reply; 19+ messages in thread From: Daniel Jacobowitz @ 2010-02-03 23:30 UTC (permalink / raw) To: Roland McGrath; +Cc: Tom Tromey, Chris Moller, Eric Bachalo, Project Archer On Wed, Feb 03, 2010 at 03:25:06PM -0800, Roland McGrath wrote: > Yeah, I was going for parity and the /c format would not have been my first > choice if we didn't already have it. I think: > > $2 = (enum frobozz) 1 = ENUM1 > > would be fine too, or maybe reverse the clauses. "(type) digit" is > probably what's most likely always to be pasteable anywhere, in case > it's into a context where you don't have the same scope for the enum > constant (in C++). Something to consider is what contexts you want this output in. I think this would get old really fast: $2 = { code = CODE_ADD = (enum code) 1, ops = OPS_TWO = (enum ops) 2 } vs $2 = { code = CODE_ADD, ops = OPS_TWO } (gdb) p CODE_ADD $3 = CODE_ADD = (enum code) 1 Hmm, can't say I like that format any better either, but there's got to be a good one. I usually p/d CODE_ADD to find out the value... My takeaway point here is to consider the overall impact. -- Daniel Jacobowitz CodeSourcery ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-03 23:30 ` Daniel Jacobowitz @ 2010-02-03 23:34 ` Chris Moller 2010-02-04 1:32 ` Tom Tromey 0 siblings, 1 reply; 19+ messages in thread From: Chris Moller @ 2010-02-03 23:34 UTC (permalink / raw) To: Daniel Jacobowitz Cc: Roland McGrath, Tom Tromey, Eric Bachalo, Project Archer On 02/03/10 18:28, Daniel Jacobowitz wrote: > On Wed, Feb 03, 2010 at 03:25:06PM -0800, Roland McGrath wrote: > >> Yeah, I was going for parity and the /c format would not have been my first >> choice if we didn't already have it. I think: >> >> $2 = (enum frobozz) 1 = ENUM1 >> >> would be fine too, or maybe reverse the clauses. "(type) digit" is >> probably what's most likely always to be pasteable anywhere, in case >> it's into a context where you don't have the same scope for the enum >> constant (in C++). >> > > Something to consider is what contexts you want this output in. I > think this would get old really fast: > > $2 = { code = CODE_ADD = (enum code) 1, ops = OPS_TWO = (enum ops) 2 } > > vs > > $2 = { code = CODE_ADD, ops = OPS_TWO } > (gdb) p CODE_ADD > $3 = CODE_ADD = (enum code) 1 > > Hmm, can't say I like that format any better either, but there's got > to be a good one. I usually p/d CODE_ADD to find out the value... > While I was looking for where this stuff is formatted, I noticed it goes through python/py-prettyprint.c which, I assume, can format things any way you like it. How about just something generic by default and use a pretty-print thingy if that doesn't work? (Uh, assuming py-prettyprint.c does that....) > My takeaway point here is to consider the overall impact. > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-03 23:34 ` Chris Moller @ 2010-02-04 1:32 ` Tom Tromey 2010-02-04 17:30 ` Chris Moller 0 siblings, 1 reply; 19+ messages in thread From: Tom Tromey @ 2010-02-04 1:32 UTC (permalink / raw) To: Chris Moller Cc: Daniel Jacobowitz, Roland McGrath, Eric Bachalo, Project Archer Chris> While I was looking for where this stuff is formatted, I noticed it Chris> goes through python/py-prettyprint.c which, I assume, can format Chris> things any way you like it. How about just something generic by Chris> default and use a pretty-print thingy if that doesn't work? (Uh, Chris> assuming py-prettyprint.c does that....) This only works if you add python code for it. I think it is preferable to print enumerators nicely by default, not as a special add-on. My advice is, read through the follow-ups and make an informed choice. Daniel makes good points. Tom ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-04 1:32 ` Tom Tromey @ 2010-02-04 17:30 ` Chris Moller 2010-02-04 23:53 ` Tom Tromey 0 siblings, 1 reply; 19+ messages in thread From: Chris Moller @ 2010-02-04 17:30 UTC (permalink / raw) To: Tom Tromey; +Cc: Daniel Jacobowitz, Roland McGrath, Project Archer Just FYI, this patch breaks a handful of existing tests: gdb.base/setvar.exp gdb.cp/classes.exp gdb.cp/m-data.exp gdb.cp/m-static.exp cp/namespace.exp gdb.mi/mi-var-display.exp gdb.python/py-value.exp None of them are hard to fix in the expects--I just wanted The World to realise that there are going to be noticeable consequences to this fairly minor nicety. It'll take me a bit to fix the t/cs--if The Powers That Be would rather this patch not proceed, I'd appreciate it if you'd let me know sooner rather than later and spare me all the fix-up work. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-04 17:30 ` Chris Moller @ 2010-02-04 23:53 ` Tom Tromey 2010-02-05 0:29 ` Chris Moller 0 siblings, 1 reply; 19+ messages in thread From: Tom Tromey @ 2010-02-04 23:53 UTC (permalink / raw) To: Chris Moller; +Cc: Daniel Jacobowitz, Roland McGrath, Project Archer Chris> Just FYI, this patch breaks a handful of existing tests: Chris> gdb.base/setvar.exp Chris> gdb.cp/classes.exp Chris> gdb.cp/m-data.exp Chris> gdb.cp/m-static.exp Chris> cp/namespace.exp Chris> gdb.mi/mi-var-display.exp Chris> gdb.python/py-value.exp Yeah. Chris> None of them are hard to fix in the expects--I just wanted The World Chris> to realise that there are going to be noticeable consequences to this Chris> fairly minor nicety. It'll take me a bit to fix the t/cs--if The Chris> Powers That Be would rather this patch not proceed, I'd appreciate it Chris> if you'd let me know sooner rather than later and spare me all the Chris> fix-up work. Usually you just have to do it and see what happens. Even if we all agree on a format here, someone else might disagree on gdb-patches, or they might think of some new reason for it to be different. I, personally, want some form of this change, because every time I type "p FOO" and get "$1 = FOO", I think, grrr, gdb could have told me what I really wanted to know. If enough people dislike it we can always add 'set print blah blah'. Tom ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-04 23:53 ` Tom Tromey @ 2010-02-05 0:29 ` Chris Moller 2010-02-05 20:19 ` Roland McGrath 0 siblings, 1 reply; 19+ messages in thread From: Chris Moller @ 2010-02-05 0:29 UTC (permalink / raw) To: Tom Tromey; +Cc: Daniel Jacobowitz, Roland McGrath, Project Archer On 02/04/10 18:53, Tom Tromey wrote: > Chris> Just FYI, this patch breaks a handful of existing tests: > Chris> gdb.base/setvar.exp > Chris> gdb.cp/classes.exp > Chris> gdb.cp/m-data.exp > Chris> gdb.cp/m-static.exp > Chris> cp/namespace.exp > Chris> gdb.mi/mi-var-display.exp > Chris> gdb.python/py-value.exp > > Yeah. > > Chris> None of them are hard to fix in the expects--I just wanted The World > Chris> to realise that there are going to be noticeable consequences to this > Chris> fairly minor nicety. It'll take me a bit to fix the t/cs--if The > Chris> Powers That Be would rather this patch not proceed, I'd appreciate it > Chris> if you'd let me know sooner rather than later and spare me all the > Chris> fix-up work. > > Usually you just have to do it and see what happens. Even if we all > agree on a format here, someone else might disagree on gdb-patches, or > they might think of some new reason for it to be different. > > I, personally, want some form of this change, because every time I type > "p FOO" and get "$1 = FOO", I think, grrr, gdb could have told me what I > really wanted to know. If enough people dislike it we can always add > 'set print blah blah'. > Based on what people were saying yesterday, I thought I'd go with $1 = Val1=(enum E)56 It looks like a legitimate C expression. Lacking spaces, it's cohesive, and it's easily understood to present the same information in multiple formats. > Tom > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-05 0:29 ` Chris Moller @ 2010-02-05 20:19 ` Roland McGrath 2010-02-05 22:52 ` Chris Moller 0 siblings, 1 reply; 19+ messages in thread From: Roland McGrath @ 2010-02-05 20:19 UTC (permalink / raw) To: Chris Moller; +Cc: Tom Tromey, Daniel Jacobowitz, Project Archer > Based on what people were saying yesterday, I thought I'd go with > > $1 = Val1=(enum E)56 > > It looks like a legitimate C expression. Lacking spaces, it's cohesive, > and it's easily understood to present the same information in multiple > formats. Please don't skimp on the spaces. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-05 20:19 ` Roland McGrath @ 2010-02-05 22:52 ` Chris Moller 0 siblings, 0 replies; 19+ messages in thread From: Chris Moller @ 2010-02-05 22:52 UTC (permalink / raw) To: archer No matter what format I use, someone won't like it, so how about this: I'll add a new "set" option, "set enum-fmt" or something like that that can take a printf-style format string. Default format if the string isn't set is current behaviour, otherwise the string is parsed, looking for %-directives, %v for the numeric value of the enum, %s for the symbolic value, and %e for the enum decl. My spaceless format, e.g., would be set with a format of set enum-fmt "%s=(enum %e)%d" If Roland wants spaces, he can use set enum-fmt "%s = (enum %e) %d" or whatever. I've never been a big fan of breaking historical behaviour, and this avoids that. It's a bigger patch, but I think it's a better one. cm On 02/05/10 15:18, Roland McGrath wrote: >> Based on what people were saying yesterday, I thought I'd go with >> >> $1 = Val1=(enum E)56 >> >> It looks like a legitimate C expression. Lacking spaces, it's cohesive, >> and it's easily understood to present the same information in multiple >> formats. >> > > Please don't skimp on the spaces. > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-03 22:48 ` Tom Tromey 2010-02-03 23:14 ` Daniel Jacobowitz @ 2010-02-03 23:21 ` Chris Moller 1 sibling, 0 replies; 19+ messages in thread From: Chris Moller @ 2010-02-03 23:21 UTC (permalink / raw) To: Tom Tromey; +Cc: Roland McGrath, Eric Bachalo, Project Archer On 02/03/10 17:48, Tom Tromey wrote: >>>>>> "Roland" == Roland McGrath<roland@redhat.com> writes: >>>>>> > > Roland> I think it should use a format consistent with p/c, e.g.: > Roland> (gdb) p 'a' > Roland> $1 = 97 'a' > Roland> (gdb) p ENUM1 > Roland> $2 = (enum frobozz) 1 ENUM1 > > Works for me. Chris, please do it this way, thanks. > Okay, a whole 5-line patch yields (gdb) p e $1 = (enum E) 56 Val1 (gdb) p Val1 $2 = (enum E) 56 Val1 (gdb) (The enum, BTW, is num E { Val1 = 56, Val2 }; because you can find a 0 any old place.) Personally, I think $2 = (enum E) 56 Val1 is a bit hard to read. I'd vote for something like $2 = (enum E)56 'Val1' But whatever... > Tom > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PR 11067 2010-02-03 21:52 ` PR 11067 Chris Moller 2010-02-03 22:43 ` Roland McGrath @ 2010-02-03 22:47 ` Tom Tromey 1 sibling, 0 replies; 19+ messages in thread From: Tom Tromey @ 2010-02-03 22:47 UTC (permalink / raw) To: Chris Moller; +Cc: Eric Bachalo, Project Archer >>>>> "Chris" == Chris Moller <cmoller@redhat.com> writes: Chris> The simple fix is to always print both the symbolic name and the Chris> decimal value: >> (gdb) p e >> $1 = Val1 (= 56) >> (gdb) p Val1 >> $2 = Val1 (= 56) >> (gdb) Chris> That's a one-line change in c-valprint.c. Trying to differentiate Chris> between those two cases looks like it would be significantly more Chris> difficult. Chris> Would the one-line change do? Or does the fix need to do exactly Chris> what the request says? The one-line fix is fine. Tom ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2010-02-05 22:52 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-02-02 22:12 patch for pr9067 Chris Moller 2010-02-02 23:28 ` Tom Tromey 2010-02-03 2:25 ` Chris Moller 2010-02-03 20:04 ` Tom Tromey [not found] ` <m3bpg7yznc.fsf@fleche.redhat.com> [not found] ` <4B68DC25.1090808@redhat.com> [not found] ` <m3fx5irwx6.fsf@fleche.redhat.com> 2010-02-03 21:52 ` PR 11067 Chris Moller 2010-02-03 22:43 ` Roland McGrath 2010-02-03 22:48 ` Tom Tromey 2010-02-03 23:14 ` Daniel Jacobowitz 2010-02-03 23:25 ` Roland McGrath 2010-02-03 23:30 ` Daniel Jacobowitz 2010-02-03 23:34 ` Chris Moller 2010-02-04 1:32 ` Tom Tromey 2010-02-04 17:30 ` Chris Moller 2010-02-04 23:53 ` Tom Tromey 2010-02-05 0:29 ` Chris Moller 2010-02-05 20:19 ` Roland McGrath 2010-02-05 22:52 ` Chris Moller 2010-02-03 23:21 ` Chris Moller 2010-02-03 22:47 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).