Re: [PATCH] Add '-at location' support to 'macro expand', 'macro expand-once', 'info macro', 'ptype', and 'whatis' commands.

Andrew Burgess
2016-08-02T11:17:43Z

David,

I'm not a maintainer so cannot approve your patch, however, I took a
look through and made a few commands. I didn't look at the docs as
Eli has done that already.

Most of the points are minor coding standard stuff, there's a couple
of scope issues, but otherwise looks great.

Thanks,
Andrew

* David Taylor <dtaylor@emc.com> [2016-07-18 10:23:56 -0400]:

> Add '-at location' support to 'macro expand', 'macro expand-once', 'info
> macro', 'ptype', and 'whatis' commands.
>
> gdb/ChangeLog:
>
> * macrocmd.c (macro_expand_command): Add support for '-at
> location' option.
> (macro_expand_once_command): Ditto.
> (info_macro_command): Ditto.
> (_initialize_macrocmd): Update doc strings.
> * typeprint.c (whatis_exp): Add support for '-at location' option.
> (_initialize_typeprint): Update doc strings.
> * NEWS: Mention enhancement to info macro, macro expand, macro
> expand-once, ptype, and whatis commands.
>
> gdb/doc/ChangeLog:
>
> * gdb.texinfo (Macros): Document '-at location' option to macro
> expand, macro expand-once, and info macro.
> (Symbols): Document the '-at location' option to whatis and ptype.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/info-macros.exp: Add tests of '-at location' option.
> * gdb.base/ptype.exp: Ditto.
> * gdb.base/whatis-exp.exp: Ditto.
> ---
> gdb/ChangeLog | 12 +++
> gdb/NEWS | 6 ++
> gdb/doc/ChangeLog | 6 ++
> gdb/doc/gdb.texinfo | 40 ++++++---
> gdb/macrocmd.c | 159 ++++++++++++++++++++++++++++-----
> gdb/testsuite/ChangeLog | 6 ++
> gdb/testsuite/gdb.base/info-macros.exp | 24 +++++
> gdb/testsuite/gdb.base/ptype.exp | 9 ++
> gdb/testsuite/gdb.base/whatis-exp.exp | 2 +-
> gdb/typeprint.c | 118 +++++++++++++++++-------
> 10 files changed, 318 insertions(+), 64 deletions(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 4139a29..e35377e 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,15 @@
> +2016-07-15 David Taylor <dtaylor@emc.com>
> +
> + * macrocmd.c (macro_expand_command): Add support for '-at
> + location' option.
> + (macro_expand_once_command): Ditto.
> + (info_macro_command): Ditto.
> + (_initialize_macrocmd): Update doc strings.
> + * typeprint.c (whatis_exp): Add support for '-at location' option.
> + (_initialize_typeprint): Update doc strings.
> + * NEWS: Mention enhancement to info macro, macro expand, macro
> + expand-once, ptype, and whatis commands.
> +
> 2016-07-12 Tom Tromey <tom@tromey.com>
>
> PR python/19293:
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 3823f20..7e42f90 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -80,6 +80,12 @@ new-ui INTERP TTY
> Start a new user interface instance running INTERP as interpreter,
> using the TTY file for input/output.
>
> +* Changes to existing commands
> +
> + The info macro, macro expand, macro expand-once, ptype, and whatis
> + commands now take an option [-at LOCATION,] for specifying the
> + location.
> +
> * Support for tracepoints and fast tracepoints on s390-linux and s390x-linux
> was added in GDBserver, including JIT compiling fast tracepoint's
> conditional expression bytecode into native code.
> diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
> index 385ca41..97675e8 100644
> --- a/gdb/doc/ChangeLog
> +++ b/gdb/doc/ChangeLog
> @@ -1,3 +1,9 @@
> +2016-07-15 David Taylor <dtaylor@emc.com>
> +
> + * gdb.texinfo (Macros): Document '-at location' option to macro
> + expand, macro expand-once, and info macro.
> + (Symbols): Document the '-at location' option to whatis and ptype.
> +
> 2016-07-12 Tom Tromey <tom@tromey.com>
>
> PR python/19293:
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index a068622..b857070 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -12160,15 +12160,18 @@ the following commands for working with macros explicitly.
> @cindex macro expansion, showing the results of preprocessor
> @cindex preprocessor macro expansion, showing the results of
> @cindex expanding preprocessor macros
> -@item macro expand @var{expression}
> +@item macro expand [-at @var{location},] [--] @var{expression}
> @itemx macro exp @var{expression}
> Show the results of expanding all preprocessor macro invocations in
> @var{expression}. Since @value{GDBN} simply expands macros, but does
> not parse the result, @var{expression} need not be a valid expression;
> -it can be any string of tokens.
> +it can be any string of tokens. If the optional @var{location} is
> +specified, it is used to determine which macro definitions are in
> +scope; otherwise, as before, it uses the source and line asociated with
> +the current program counter.
>
> @kindex macro exp1
> -@item macro expand-once @var{expression}
> +@item macro expand-once [-at @var{location},] [--] @var{expression}
> @itemx macro exp1 @var{expression}
> @cindex expand macro once
> @i{(This command is not yet implemented.)} Show the results of
> @@ -12178,18 +12181,24 @@ left unchanged. This command allows you to see the effect of a
> particular macro more clearly, without being confused by further
> expansions. Since @value{GDBN} simply expands macros, but does not
> parse the result, @var{expression} need not be a valid expression; it
> -can be any string of tokens.
> +can be any string of tokens. If the optional @var{location} is
> +specified, it is used to determine which macro definitions are in
> +scope; otherwise, as before, it uses the source and line asociated with
> +the current program counter.
>
> @kindex info macro
> @cindex macro definition, showing
> @cindex definition of a macro, showing
> @cindex macros, from debug info
> -@item info macro [-a|-all] [--] @var{macro}
> +@item info macro [-a|-all] [-at LOCATION,] [--] @var{macro}
> Show the current definition or all definitions of the named @var{macro},
> and describe the source location or compiler command-line where that
> -definition was established. The optional double dash is to signify the end of
> -argument processing and the beginning of @var{macro} for non C-like macros where
> -the macro may begin with a hyphen.
> +definition was established. The optional double dash is to signify the
> +end of argument processing and the beginning of @var{macro} for non
> +C-like macros where the macro may begin with a hyphen. If the optional
> +@var{location} is specified, it is used to determine which definition,
> +if any, of the macro is in scope; otherwise, as before, it uses the
> +source and line asociated with the current program counter.
>
> @kindex info macros
> @item info macros @var{location}
> @@ -16885,7 +16894,7 @@ The parameter @code{demangle-style} specifies how to interpret the kind
> of mangling used. @xref{Print Settings}.
>
> @kindex whatis
> -@item whatis[/@var{flags}] [@var{arg}]
> +@item whatis[/@var{flags}] [-at @var{location},] [--] [@var{arg}]
> Print the data type of @var{arg}, which can be either an expression
> or a name of a data type. With no argument, print the data type of
> @code{$}, the last value in the value history.
> @@ -16941,8 +16950,19 @@ Print typedefs defined in the class. This is the default, but the flag
> exists in case you change the default with @command{set print type typedefs}.
> @end table
>
> +Options are:
> +
> +@table @code
> +@item -at @var{location},
> +Specifies an alternate location @var{location} to use for lookup
> +instead of the current stack frame.
> +
> +@item --
> +Specifies the end of options.
> +@end table
> +
> @kindex ptype
> -@item ptype[/@var{flags}] [@var{arg}]
> +@item ptype[/@var{flags}] [-at @var{location},] [--] [@var{arg}]
> @code{ptype} accepts the same arguments as @code{whatis}, but prints a
> detailed description of the type, instead of just the name of the type.
> @xref{Expressions, ,Expressions}.
> diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
> index 7661336..d5d2c32 100644
> --- a/gdb/macrocmd.c
> +++ b/gdb/macrocmd.c
> @@ -26,6 +26,7 @@
> #include "command.h"
> #include "gdbcmd.h"
> #include "linespec.h"
> +#include "location.h"
>
> \f
> /* The `macro' prefix command. */
> @@ -58,9 +59,48 @@ macro_expand_command (char *exp, int from_tty)
> struct macro_scope *ms = NULL;
> char *expanded = NULL;
> struct cleanup *cleanup_chain = make_cleanup (free_current_contents, &ms);
> + int processing_args = 1;
> + int location_specified = 0;
> + struct linespec_result canonical;
> + struct event_location *location;

The location_specified is never used, only set.

You can move 'location' into the scope in which its used.

You could also move canonical into the scope in which it is used, if
you ran the clean up chain within that scope too. You have done this
for 'info_macro_command' but not for 'macro_expand_command' or
'macro_expand_once_command'.

I think running the cleanups in the local scope is the nicest, but I
guess either is fine. It might be nice if you took one approach or
the other across the whole patch though.... unless I'm missing
a reason why there's a difference (more than likely).

>
> make_cleanup (free_current_contents, &expanded);
>
> + while (processing_args && exp && *exp == '-')

The coding standard says pointers must be compared to NULL, so 'exp !=
NULL'.

> + {
> + if (check_for_argument (&exp, "-at", sizeof ("-at") - 1))
> + {
> + location_specified = 1;
> +
> + exp = skip_spaces (exp);
> + init_linespec_result (&canonical);
> + location = new_linespec_location (&exp);
> + make_cleanup_delete_event_location (location);
> + decode_line_full (location, DECODE_LINE_FUNFIRSTLINE, NULL,
> + (struct symtab *) NULL, 0, &canonical,
> + NULL, NULL);
> + make_cleanup_destroy_linespec_result (&canonical);
> + exp = skip_spaces (exp);
> + if (exp[0] == ',')
> + {
> + exp++;
> + exp = skip_spaces (exp);
> + }
> + ms = sal_macro_scope (canonical.sals->vec->sals.sals[0]);
> + make_cleanup (free_current_contents, &ms);
> + }
> + else if (check_for_argument (&exp, "--", sizeof ("--") - 1))
> + /* Our macro support seems rather C specific but this would
> + seem necessary for languages allowing - in macro names.
> + e.g. Scheme's (defmacro ->foo () "bar\n") */
> + processing_args = 0;
> + else
> + {
> + error (_("Unrecognized option '%s' to macro expand command. "
> + "Try \"help macro expand\"."), exp);
> + }
> + exp = skip_spaces (exp);
> + }
> /* You know, when the user doesn't specify any expression, it would be
> really cool if this defaulted to the last expression evaluated.
> Then it would be easy to ask, "Hey, what did I just evaluate?" But
> @@ -71,7 +111,9 @@ macro_expand_command (char *exp, int from_tty)
> " expression you\n"
> "want to expand."));
>
> - ms = default_macro_scope ();
> + if (! ms)

Change to 'ms == NULL'.

> + ms = default_macro_scope ();
> +
> if (ms)
> {
> expanded = macro_expand (exp, standard_macro_lookup, ms);
> @@ -93,8 +135,49 @@ macro_expand_once_command (char *exp, int from_tty)
> struct macro_scope *ms = NULL;
> char *expanded = NULL;
> struct cleanup *cleanup_chain = make_cleanup (free_current_contents, &ms);
> + int processing_args = 1;
> + int location_specified = 0;
> + struct linespec_result canonical;
> + struct event_location *location;

Again, location_specified is set but never used, and 'location' can be
moved into the tighter scope. As with the above there's the
possibility of moving 'canonical' too.

> +
> make_cleanup (free_current_contents, &expanded);
>
> + while (processing_args && exp && *exp == '-')

Again, should be 'exp != NULL'.

> + {
> + if (check_for_argument (&exp, "-at", sizeof ("-at") - 1))
> + {
> + location_specified = 1;
> +
> + exp = skip_spaces (exp);
> + init_linespec_result (&canonical);
> + location = new_linespec_location (&exp);
> + make_cleanup_delete_event_location (location);
> + decode_line_full (location, DECODE_LINE_FUNFIRSTLINE, NULL,
> + (struct symtab *) NULL, 0, &canonical,
> + NULL, NULL);
> + make_cleanup_destroy_linespec_result (&canonical);
> + exp = skip_spaces (exp);
> + if (exp[0] == ',')
> + {
> + exp++;
> + exp = skip_spaces (exp);
> + }
> + ms = sal_macro_scope (canonical.sals->vec->sals.sals[0]);
> + make_cleanup (free_current_contents, &ms);
> + }
> + else if (check_for_argument (&exp, "--", sizeof ("--") - 1))
> + /* Our macro support seems rather C specific but this would
> + seem necessary for languages allowing - in macro names.
> + e.g. Scheme's (defmacro ->foo () "bar\n") */
> + processing_args = 0;
> + else
> + {
> + error (_("Unrecognized option '%s' to macro expand command. "
> + "Try \"help macro expand\"."), exp);
> + }
> + exp = skip_spaces (exp);
> + }
> +
> /* You know, when the user doesn't specify any expression, it would be
> really cool if this defaulted to the last expression evaluated.
> And it should set the once-expanded text as the new `last
> @@ -105,7 +188,9 @@ macro_expand_once_command (char *exp, int from_tty)
> " the expression\n"
> "you want to expand."));
>
> - ms = default_macro_scope ();
> + if (! ms)

Compare to NULL again.

> + ms = default_macro_scope ();
> +
> if (ms)
> {
> expanded = macro_expand_once (exp, standard_macro_lookup, ms);
> @@ -205,45 +290,70 @@ static void
> info_macro_command (char *args, int from_tty)
> {
> struct macro_scope *ms = NULL;
> - struct cleanup *cleanup_chain;
> + struct cleanup *cleanup_chain = NULL;
> char *name;
> int show_all_macros_named = 0;
> char *arg_start = args;
> int processing_args = 1;
> + int location_specified = 0;

Here location_specified _is_ used, but I think it probably shouldn't
be. In the above functions you use 'ms == NULL' in place of
location_specified and that feels like a better choice. I'd stick to
that and drop location_specified here too.

>
> - while (processing_args
> - && arg_start && *arg_start == '-' && *arg_start != '\0')
> + while (processing_args && args && *args == '-')

NULL compare for args.

> {
> - char *p = skip_to_space (arg_start);
> -
> - if (strncmp (arg_start, "-a", p - arg_start) == 0
> - || strncmp (arg_start, "-all", p - arg_start) == 0)
> + if (check_for_argument (&args, "-a", sizeof ("-a") - 1)
> + || check_for_argument (&args, "-all", sizeof ("-all") - 1))
> show_all_macros_named = 1;
> - else if (strncmp (arg_start, "--", p - arg_start) == 0)
> + else if (check_for_argument (&args, "-at", sizeof ("-at") - 1))
> + {
> + struct linespec_result canonical;
> + struct event_location *location;
> +
> + location_specified = 1;
> +
> + args = skip_spaces (args);
> + init_linespec_result (&canonical);
> + location = new_linespec_location (&args);
> + make_cleanup_delete_event_location (location);
> + decode_line_full (location, DECODE_LINE_FUNFIRSTLINE, NULL,
> + (struct symtab *) NULL, 0, &canonical,
> + NULL, NULL);
> +
> + cleanup_chain = make_cleanup_destroy_linespec_result (&canonical);
> + args = skip_spaces (args);
> + if (args[0] == ',')
> + {
> + args++;
> + args = skip_spaces (args);
> + }
> + ms = sal_macro_scope (canonical.sals->vec->sals.sals[0]);
> + do_cleanups (cleanup_chain);
> + cleanup_chain = make_cleanup (free_current_contents, &ms);
> + }
> + else if (check_for_argument (&args, "--", sizeof ("--") - 1))
> /* Our macro support seems rather C specific but this would
> seem necessary for languages allowing - in macro names.
> e.g. Scheme's (defmacro ->foo () "bar\n") */
> processing_args = 0;
> else
> {
> - /* Relies on modified 'args' not making it in to history */
> - *p = '\0';
> error (_("Unrecognized option '%s' to info macro command. "
> - "Try \"help info macro\"."), arg_start);
> + "Try \"help info macro\"."), args);
> }
>
> - arg_start = skip_spaces (p);
> + args = skip_spaces (args);
> }
>
> - name = arg_start;
> + name = skip_spaces (args);
>
> if (! name || ! *name)
> error (_("You must follow the `info macro' command with the name"
> " of the macro\n"
> "whose definition you want to see."));
>
> - ms = default_macro_scope ();
> - cleanup_chain = make_cleanup (free_current_contents, &ms);
> + if (! location_specified)
> + {
> + ms = default_macro_scope ();
> + cleanup_chain = make_cleanup (free_current_contents, &ms);
> + }
>
> if (! ms)
> macro_inform_no_debuginfo ();
> @@ -505,7 +615,11 @@ _initialize_macrocmd (void)
>
> add_cmd ("expand", no_class, macro_expand_command, _("\
> Fully expand any C/C++ preprocessor macro invocations in EXPRESSION.\n\
> -Show the expanded expression."),
> +Show the expanded expression.\n\
> +Usage: macro expand [-at LCOATION,] [--] EXPR\n\
> +Options:\n\
> + -at Use LOCATION rather than the current PC for selecting macros.\n\
> + -- Specify the end of arguments and the beginning of the EXPR."),
> &macrolist);
> add_alias_cmd ("exp", "expand", no_class, 1, &macrolist);
> add_cmd ("expand-once", no_class, macro_expand_once_command, _("\
> @@ -518,16 +632,21 @@ introduces further macro invocations, those are left unexpanded.\n\
> \n\
> `macro expand-once' helps you see how a particular macro expands,\n\
> whereas `macro expand' shows you how all the macros involved in an\n\
> -expression work together to yield a pre-processed expression."),
> +expression work together to yield a pre-processed expression.\n\
> +Usage: macro expand-once [-at LCOATION,] [--] EXPR\n\
> +Options:\n\
> + -at Use LOCATION rather than the current PC for selecting macros.\n\
> + -- Specify the end of arguments and the beginning of the EXPR."),
> &macrolist);
> add_alias_cmd ("exp1", "expand-once", no_class, 1, &macrolist);
>
> add_info ("macro", info_macro_command,
> _("Show the definition of MACRO, and it's source location.\n\
> -Usage: info macro [-a|-all] [--] MACRO\n\
> +Usage: info macro [-a|-all] [-at LOCATION,] [--] MACRO\n\
> Options: \n\
> -a, --all Output all definitions of MACRO in the current compilation\
> unit.\n\
> + -at Use LOCATION rather than the current PC for selecting macros.\n\
> -- Specify the end of arguments and the beginning of the MACRO."));
>
> add_info ("macros", info_macros_command,
> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
> index 7ab1228..dea618a 100644
> --- a/gdb/testsuite/ChangeLog
> +++ b/gdb/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2016-07-15 David Taylor <dtaylor@emc.com>
> +
> + * gdb.base/info-macros.exp: Add tests of '-at location' option.
> + * gdb.base/ptype.exp: Ditto.
> + * gdb.base/whatis-exp.exp: Ditto.
> +
> 2016-07-13 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * gdb.dwarf2/atomic-type.exp: Use function_range for low_pc and high_pc.
> diff --git a/gdb/testsuite/gdb.base/info-macros.exp b/gdb/testsuite/gdb.base/info-macros.exp
> index a798b37..7d83a8b 100644
> --- a/gdb/testsuite/gdb.base/info-macros.exp
> +++ b/gdb/testsuite/gdb.base/info-macros.exp
> @@ -133,6 +133,30 @@ set test "info macro -a -- FOO"
> set testname "$test"
> gdb_test "$test" "$r1$r2$r3$r4" "$testname"
>
> +set test "info macro -at LOCATION, FOO"
> +
> +set r5 ".* has no definition .*\r\nat .*$srcfile:\[0-9\]+"
> +set testname "$test 1"
> +gdb_test "info macro -at info-macros.c:42, FOO" "$r1" "$testname"
> +
> +set testname "$test 2"
> +gdb_test "info macro -at info-macros.c:46, FOO" "$r2" "$testname"
> +
> +set testname "$test 3"
> +gdb_test "info macro -at info-macros.c:50, FOO" "$r3" "$testname"
> +
> +set testname "$test 4"
> +gdb_test "info macro -at info-macros.c:54, FOO" "$r2" "$testname"
> +
> +set testname "$test 5"
> +gdb_test "info macro -at info-macros.c:58, FOO" "$r1" "$testname"
> +
> +set testname "$test 6"
> +gdb_test "info macro -at info-macros.c:62, FOO" "$r5" "$testname"
> +
> +set testname "$test 7"
> +gdb_test "info macro -at info-macros.c:66, FOO" "$r4" "$testname"
> +
> set test "info macros"
> set r1 ".*#define FOO \"hello\""
> set r2 ".*#define ONE"
> diff --git a/gdb/testsuite/gdb.base/ptype.exp b/gdb/testsuite/gdb.base/ptype.exp
> index dd61091..0f0aaa7 100644
> --- a/gdb/testsuite/gdb.base/ptype.exp
> +++ b/gdb/testsuite/gdb.base/ptype.exp
> @@ -584,6 +584,15 @@ gdb_test "ptype foo" "type = int" "ptype foo typedef after second list of intfoo
> gdb_test "list charfoo" ".*"
> gdb_test "ptype foo" "type = char" "ptype foo typedef after second list of charfoo"
>
> +# Test printing type of typedefs in different scopes, with same name
> +# but different type -- this time specifying the location explicitly
> +# rather than implicitly via 'list'.
> +
> +gdb_test "ptype -at intfoo, foo" "type = int" "ptype foo typedef -at intfoo"
> +gdb_test "ptype -at charfoo, foo" "type = char" "ptype foo typedef -at charfoo"
> +#gdb_test "ptype -at intfoo foo" "type = int" "ptype foo typedef after second list of intfoo"
> +#gdb_test "ptype foo" "type = char" "ptype foo typedef after second list of charfoo"
> +
> # Test printing type of string constants and array constants, but
> # requires a running process. These call malloc, and can take a long
> # time to execute over a slow serial link, so increase the timeout.
> diff --git a/gdb/testsuite/gdb.base/whatis-exp.exp b/gdb/testsuite/gdb.base/whatis-exp.exp
> index 511d490..9522ae8 100644
> --- a/gdb/testsuite/gdb.base/whatis-exp.exp
> +++ b/gdb/testsuite/gdb.base/whatis-exp.exp
> @@ -65,7 +65,7 @@ gdb_test "whatis x=y" "type = int" "whatis value of x=y"
>
> gdb_test "whatis x+=2" "type = int" "whatis value of x+=2"
> gdb_test "whatis ++x" "type = int" "whatis value of ++x"
> -gdb_test "whatis --x" "type = int" "whatis value of --x"
> +gdb_test "whatis -- --x" "type = int" "whatis value of --x"
> gdb_test "whatis x++" "type = int" "whatis value of x++"
> gdb_test "whatis x--" "type = int" "whatis value of x--"
>
> diff --git a/gdb/typeprint.c b/gdb/typeprint.c
> index e77513e..6bad526 100644
> --- a/gdb/typeprint.c
> +++ b/gdb/typeprint.c
> @@ -36,6 +36,9 @@
> #include "cli/cli-utils.h"
> #include "extension.h"
> #include "completer.h"
> +#include "linespec.h"
> +#include "block.h"
> +#include "location.h"
>
> extern void _initialize_typeprint (void);
>
> @@ -410,50 +413,96 @@ whatis_exp (char *exp, int show)
> int using_enc = 0;
> struct value_print_options opts;
> struct type_print_options flags = default_ptype_flags;
> + int processing_args = 1;
> + int location_specified = 0; /* was '-at <location>' specified */
> + CORE_ADDR pc = 0;
> + struct linespec_result canonical;
> + struct event_location *location;
>
> old_chain = make_cleanup (null_cleanup, NULL);
>
> if (exp)
> {
> - if (*exp == '/')
> + while (processing_args && ((*exp == '/') || (*exp == '-')))
> {
> - int seen_one = 0;
> -
> - for (++exp; *exp && !isspace (*exp); ++exp)
> + if (*exp == '/')
> {
> - switch (*exp)
> + int seen_one = 0;
> +
> + for (++exp; *exp && !isspace (*exp); ++exp)
> {
> - case 'r':
> - flags.raw = 1;
> - break;
> - case 'm':
> - flags.print_methods = 0;
> - break;
> - case 'M':
> - flags.print_methods = 1;
> - break;
> - case 't':
> - flags.print_typedefs = 0;
> - break;
> - case 'T':
> - flags.print_typedefs = 1;
> - break;
> - default:
> - error (_("unrecognized flag '%c'"), *exp);
> + switch (*exp)
> + {
> + case 'r':
> + flags.raw = 1;
> + break;
> + case 'm':
> + flags.print_methods = 0;
> + break;
> + case 'M':
> + flags.print_methods = 1;
> + break;
> + case 't':
> + flags.print_typedefs = 0;
> + break;
> + case 'T':
> + flags.print_typedefs = 1;
> + break;
> + default:
> + error (_("unrecognized flag '%c'"), *exp);
> + }
> + seen_one = 1;
> }
> - seen_one = 1;
> +
> + if (!*exp && !seen_one)
> + error (_("flag expected"));
> + if (!isspace (*exp))
> + error (_("expected space after format"));
> }
> + else if (check_for_argument (&exp, "-at", sizeof ("-at") -1))
> + {
> + location_specified = 1;
> + exp = skip_spaces (exp);
> + init_linespec_result (&canonical);
> + location = new_linespec_location (&exp);
> + make_cleanup_delete_event_location (location);
> + decode_line_full (location, DECODE_LINE_FUNFIRSTLINE, NULL,
> + (struct symtab *) NULL, 0, &canonical,
> + NULL, NULL);
> + make_cleanup_destroy_linespec_result (&canonical);
> + exp = skip_spaces (exp);
> + if (exp[0] == ',')
> + {
> + exp++;
> + exp = skip_spaces (exp);
> + }
> + if (VEC_empty (linespec_sals, canonical.sals))
> + error ("unable to get pc associated with location");
>
> - if (!*exp && !seen_one)
> - error (_("flag expected"));
> - if (!isspace (*exp))
> - error (_("expected space after format"));
> + pc = VEC_index (linespec_sals, canonical.sals, 0)->sals.sals[0].pc;
> + }
> + else if (check_for_argument (&exp, "--", sizeof ("--") -1))
> + processing_args = 0;
> + else
> + {
> + error (_("Unrecognized option '%s' to ptype command. "
> + "Try \"help ptype\"."), exp);
> + }
> exp = skip_spaces (exp);
> }
> -
> - expr = parse_expression (exp);
> - make_cleanup (free_current_contents, &expr);
> - val = evaluate_type (expr);
> + if (location_specified)
> + {
> + const char *arg = exp;
> + expr = parse_exp_1 (&arg, pc, block_for_pc (pc), 0);
> + make_cleanup (free_current_contents, &expr);
> + val = evaluate_type (expr);
> + }
> + else
> + {
> + expr = parse_expression (exp);
> + make_cleanup (free_current_contents, &expr);
> + val = evaluate_type (expr);
> + }

I wonder if it might be nicer to move the make_cleanup and
evaluate_type call out of the if/else and so reduce the duplication?

> }
> else
> val = access_value_history (0);
> @@ -685,7 +734,7 @@ _initialize_typeprint (void)
>
> c = add_com ("ptype", class_vars, ptype_command, _("\
> Print definition of type TYPE.\n\
> -Usage: ptype[/FLAGS] TYPE | EXPRESSION\n\
> +Usage: ptype[/FLAGS] [-at LOCATION,] [--] TYPE | EXPRESSION\n\
> Argument may be any type (for example a type name defined by typedef,\n\
> or \"struct STRUCT-TAG\" or \"class CLASS-NAME\" or \"union UNION-TAG\"\n\
> or \"enum ENUM-TAG\") or an expression.\n\
> @@ -697,7 +746,10 @@ Available FLAGS are:\n\
> /m do not print methods defined in a class\n\
> /M print methods defined in a class\n\
> /t do not print typedefs defined in a class\n\
> - /T print typedefs defined in a class"));
> + /T print typedefs defined in a class\n\
> +Options:\n\
> + -at Use LOCATION rather than the selected stack frame to look up the name.\n\
> + -- Specify the end of options and the start of the TYPE or EXPRESSION."));
> set_cmd_completer (c, expression_completer);
>
> c = add_com ("whatis", class_vars, whatis_command,
> --
> 1.9.1
>