From patchwork Sun Nov 23 20:00:52 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Evans X-Patchwork-Id: 3875 Received: (qmail 4815 invoked by alias); 23 Nov 2014 20:00:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 4797 invoked by uid 89); 23 Nov 2014 20:00:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.7 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPAM_SUBJECT1, SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-wi0-f181.google.com Received: from mail-wi0-f181.google.com (HELO mail-wi0-f181.google.com) (209.85.212.181) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Sun, 23 Nov 2014 20:00:56 +0000 Received: by mail-wi0-f181.google.com with SMTP id r20so3886363wiv.2 for ; Sun, 23 Nov 2014 12:00:52 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.194.95.100 with SMTP id dj4mr26685162wjb.48.1416772852664; Sun, 23 Nov 2014 12:00:52 -0800 (PST) Received: by 10.27.132.70 with HTTP; Sun, 23 Nov 2014 12:00:52 -0800 (PST) In-Reply-To: References: <001a1132e59ca4575e0508413955@google.com> <12971.1416495464@usendtaylorx2l> Date: Sun, 23 Nov 2014 12:00:52 -0800 Message-ID: Subject: Re: Fwd: Delivery Status Notification (Failure) From: Doug Evans To: dtaylor@emc.com Cc: "gdb-patches@sourceware.org" , Eli Zaretskii , Keith Seitz X-IsSubscribed: yes On Sun, Nov 23, 2014 at 11:18 AM, Doug Evans wrote: > [+ keiths, since this is linespec related, and I know > how much he loves linespecs! :-)] > > On Thu, Nov 20, 2014 at 6:57 AM, David Taylor > wrote: >> Doug Evans wrote: >>> The "," in "-at LOCATION," seems a bit random, relative to other commands. >>> >>> Maybe it is the best way to go. >>> If so, I'd like to see the reasons why it exists documented in the code. >>> >>> Can we just remove the , and require -- when necessary? >> >> It marks the end of the location and the start of the macro. It is >> patterned after >> >> maint agent [-at location,] EXPRESSION >> and >> maint agent-eval [-at location,] EXPRESSION >> >> I did it the same way for consistency. > > Ah. Can't fault that. :-) > > Still, I'm more ok with u/i visible hacks in maint commands > than with normal commands. > Bubbling this up into non-maint commands means needing > to now worry about consistency with all the other commands. > >> It seems unnecessary, but the location parsing code stops at comma but >> not at space. Presumably that is so that file names can contain spaces. >> But, that is just a guess on my part. I personally dislike file names >> containing spaces. > > Such names need to be quoted. > OTOH c++ functions with parameters, for example, > don't (usually) need to be quoted. > > linespec.c has this: > static const char * const linespec_keywords[] = { "if", "thread", "task" }; > > This may not be the best solution, and it might involve a few more > changes to linespec.c (or elsewhere) but that's one alternative: > static const char * const linespec_keywords[] = { "if", "thread", > "task", "--" }; > > It works, but "--" wasn't intended for this purpose (mark the > end of the linespec), so it's just a not-well-thought-out-idea. > > I tried to think of a situation where using a comma here would > lead to u/i warts later, but couldn't. > E.g., if the comma gets used in other contexts > then will users start complaining that "b foo, thread 1" doesn't work? > If that day comes we *could* allow the comma in that context. > But it would be odd that a comma is *required* in some > contexts and not in others. > > In the end, I'm ok with the patch, but > I think the docs (both offline and online) need to highlight the comma > as being a requirement for "-at" since it's not intuitive. > Plus I think we need to document in some linespec-related > place that "," is now required to be recognized as ending a linespec. > I don't know if that exists today. > [FAOD, a "," within a linespec is still ok, > e.g., "info macro -at foo(int, int), BAR"] > Ok with those changes. Re: Plus I think we need to document in some linespec-related place that "," is now required to be recognized as ending a linespec. FAOD, I was thinking of, say, adding a comment to the function-comment for decode_line_full in linespec.h. E.g., or some such. The point being to make the comma terminator being part of the spec of what's a valid linespec. --- IWBN to "fix" the linespec parsing machinery to recognize the end of the line spec so that the comma wasn't required. I don't want to make that a prerequisite for this patch, but down the road if that happens then the comma could just become optional. --- linespec.h= 2014-11-23 10:30:23.566784217 -0800 +++ linespec.h 2014-11-23 11:55:35.531659067 -0800 @@ -133,7 +133,11 @@ extern struct symtabs_and_lines entry describing all the matching locations. If FILTER is non-NULL, then only locations whose canonical name is equal (in the strcmp sense) to FILTER will be returned; all others will be - filtered out. */ + filtered out. + + Note for new callers: If you need a way to mark the end of the linespec + in *ARGPTR, fyi the "info macro -at LOCATION, MACRO" command uses a comma. + A comma within the linespec is still ok (e.g., "foo (int, int)"). */ extern void decode_line_full (char **argptr, int flags, struct symtab *default_symtab, int default_line,