From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 113548 invoked by alias); 25 May 2019 10:10:24 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 113540 invoked by uid 89); 25 May 2019 10:10:24 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-14.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy= X-HELO: mail-wr1-f68.google.com Received: from mail-wr1-f68.google.com (HELO mail-wr1-f68.google.com) (209.85.221.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 25 May 2019 10:10:22 +0000 Received: by mail-wr1-f68.google.com with SMTP id f8so12277711wrt.1 for ; Sat, 25 May 2019 03:10:22 -0700 (PDT) Return-Path: Received: from [192.168.1.34] ([62.28.162.51]) by smtp.gmail.com with ESMTPSA id y12sm3346203wrh.40.2019.05.25.03.10.19 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Sat, 25 May 2019 03:10:19 -0700 (PDT) Subject: Re: [PATCH 13/24] Make "print" and "compile print" support -OPT options To: Sergio Durigan Junior References: <20190522205327.2568-1-palves@redhat.com> <20190522205327.2568-14-palves@redhat.com> <87pno7ocpk.fsf@redhat.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <8564ae6d-7e46-7317-8636-eefe97db7814@redhat.com> Date: Sat, 25 May 2019 10:10:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <87pno7ocpk.fsf@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-05/txt/msg00575.txt.bz2 On 5/24/19 8:48 PM, Sergio Durigan Junior wrote: > On Wednesday, May 22 2019, Pedro Alves wrote: > >> This patch adds support for "print -option optval --", etc. >> Likewise for "compile print". >> >> We'll get: >> >> ~~~~~~ >> (gdb) help print >> Print value of expression EXP. >> Usage: print [[OPTION]... --] [/FMT] [EXP] >> >> Options: >> -address [on|off] >> Set printing of addresses. >> >> -array [on|off] >> Set pretty formatting of arrays. >> >> -array-indexes [on|off] >> Set printing of array indexes. >> >> -elements NUMBER|unlimited >> Set limit on string chars or array elements to print. >> "unlimited" causes there to be no limit. >> >> -max-depth NUMBER|unlimited >> Set maximum print depth for nested structures, unions and arrays. >> When structures, unions, or arrays are nested beyond this depth then they >> will be replaced with either '{...}' or '(...)' depending on the language. >> Use "unlimited" to print the complete structure. >> >> -null-stop [on|off] >> Set printing of char arrays to stop at first null char. >> >> -object [on|off] >> Set printing of C++ virtual function tables. >> >> -pretty [on|off] >> Set pretty formatting of structures. >> >> -repeats NUMBER|unlimited >> Set threshold for repeated print elements. >> "unlimited" causes all elements to be individually printed. >> >> -static-members [on|off] >> Set printing of C++ static members. >> >> -symbol [on|off] >> Set printing of symbol names when printing pointers. >> >> -union [on|off] >> Set printing of unions interior to structures. >> >> -vtbl [on|off] >> Set printing of C++ virtual function tables. >> >> Note: because this command accepts arbitrary expressions, if you >> specify any command option, you must use a double dash ("--") >> to mark the end of option processing. E.g.: "print -o -- myobj". >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> I want to highlight the comment above about "--". > > Thanks for the patch. I read it, and it seems like a very good > improvement. I just have a small point to make. Since: > > 1) There was a visible change of behaviour (i.e., we will now require > the user to specify "--" when she wants to end the option processing). Right. Though I wouldn't call that a change of behavior, because before there was no options support at all. > > 2) The comment above explains this change very well. > > 3) It has been replicated over all of the commands that have this > requirement, Right. It's only "print" and "compile print" so far, though. > > then why don't you make the build_help routine always print it if we're > dealing with gdb::option::PROCESS_OPTIONS_REQUIRE_DELIMITER command > types? I think it is a nice addition to the help text. I had thought of that but didn't do it because I thought that each command would want to spell it out a little bit differently. E.g., for "print" and "compile print" the difference is in the example: Note: because this command accepts arbitrary expressions, if you specify any command option, you must use a double dash ("--") to mark the end of option processing. E.g.: "print -o -- myobj". Note: because this command accepts arbitrary expressions, if you specify any command option, you must use a double dash ("--") to mark the end of option processing. E.g.: "compile print -o -- myobj". ... and I don't know whether "arbitrary expressions" will be the reason for using PROCESS_OPTIONS_REQUIRE_DELIMITER with other commands. Since there's only two cases, I didn't think it'd be worth it to try to come up with an abstraction until we have more uses? BTW, one command that currently accepts "-" options _and_ works with arbitrary expressions is the watch command: (gdb) help watch Set a watchpoint for an expression. Usage: watch [-l|-location] EXPRESSION Note how it's impossible to watch an expression named "-l". We could solve that by supporting "--" in the watch command, so you'd type: (gdb) watch -- -l So it could be argued that "watch" should be a PROCESS_OPTIONS_REQUIRE_DELIMITER just like "print", since it has the exact same issues. It happens to only support one option today, and probably watching "-FOO" is rare, so nobody ever noticed this... I _think_ that the risk of not requiring "--" for "print" is higher than for other commands. But that's my opinion, maybe others would be OK with requiring users to type: (gdb) print -- -r where they typed today: (gdb) print -r I'm stressing this issue because we're likely going to be stuck with whatever we decide, practically forever. :-) Thanks, Pedro Alves