From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15048 invoked by alias); 9 Oct 2008 14:05:10 -0000 Received: (qmail 14987 invoked by uid 22791); 9 Oct 2008 14:05:05 -0000 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 09 Oct 2008 14:04:28 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 64EF11EE106 for ; Thu, 9 Oct 2008 10:04:26 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id nlFw95pcNe+1 for ; Thu, 9 Oct 2008 10:04:26 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 0DFB42A9643 for ; Thu, 9 Oct 2008 10:04:26 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 80DC2E7ACD; Thu, 9 Oct 2008 10:04:24 -0400 (EDT) Date: Thu, 09 Oct 2008 14:05:00 -0000 From: Joel Brobecker To: gdb-patches@sourceware.org Subject: [RFC/RFA] add struct parse_context to all command functions Message-ID: <20081009140424.GD5372@adacore.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="vkogqOf2sHV7VnPd" Content-Disposition: inline User-Agent: Mutt/1.4.2.2i 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 X-SW-Source: 2008-10/txt/msg00284.txt.bz2 --vkogqOf2sHV7VnPd Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-length: 4194 Hello, I'd like to start working on adding a struct parse_context again. The ultimate goal is to pass this structure as an argument to all the "parse..." routines, rather than rely on the current_language and input_radix globals. In addition to being cleaner, it will also help fix a bug where the current_language is switched under us while trying to do parse an expression. The first thing was to define the parse_context struct. Since a very large number of .c files add new commands, I decided to put the struct in defs.h. The alternative was to put it in expression.h, besides the parse_* routines, or perhaps in command.h, or in language.h. Putting it in defs.h minimizes the size of the patch by avoiding an extra include. The next step for this task is to add a struct parse_context parameter to the "command" functions (the functions being called when the user calls a function in the UI). When the user enters a command, we know that we can use the current language and input radix to parse the arguments. So we create a struct parse context there, and pass it to the command function. The goal is to eventually have only one place that uses the parse context globals: the UI interpreter that calls the function commands. Note that I decided to pass a struct rather than a pointer to the struct to avoid any memory management issue. I plan to propagate the parse_context argument progressively. But the first step, which is to add the parse_context argument to the command functions, is massive. It affects almost every .c file in the project, so I'd like to make sure that everyone is happy with what the change looks like. For this, I will attach a patch that changes the "add_cmd" et al interface to add the parse_context argument to the command functions. The patch also contains the associated adjustements in the implementation of these functions, as well as one random .c file updated to include the new parameter. My goal is to get an agreement on the principle, then spent the time to do the entire transition, and then commit either immediately or shortly after that. I don't mind posting the patch and let it sit for a few days, but it's going to be massive, and I don't think that this is going to bring much value. But I'm ok with doing that if people prefer. If you look at the patch, you'll probably notice a few things that I would like to fix before doing the transition for real: 1. The prototypes for the various add_* functions are duplicated. There is a copy in command.h, and another in cli/cli-decode.h. Is there a specific reason why we can't keep only one copy? For instance, only keep the copy in cli/cli-decode.h? 2. Most if not all add_ command define the profile of the callback on the fly, like this: | add_prefix_cmd (char *, enum command_class, !!->| void (*fun) (char *, int, struct parse_context), | char *, struct cmd_list_element **, char *, int, | struct cmd_list_element **); There is the following typedef: | typedef void cmd_cfunc_ftype (char *args, ...) We should really use this typedef instead of repeating it everywhere. 3. There is one function "add_com" which seems to be a shortcut for add_cmd: add_com (char *name, enum command_class class, void (*fun) (char *, int, struct parse_context), char *doc) { return add_cmd (name, class, fun, doc, &cmdlist); } The only difference is that one doesn't have to pass the cmdlist to add_com. Do we really want to keep it? I have always been confused trying to find the difference between the two... 4. I noticed a few command functions that are declared global and yet should be static. I checked insight, and there are only used locally. I will fix them separately. Voila voila. I will start working on the little issues mentioned above while waiting for feedback. I would like to start working on this no later than, say, a week from now. And I will provide gdbint documentation as well as requested by Eli when we discussed this idea the last time. -- Joel --vkogqOf2sHV7VnPd Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="parse_context.diff" Content-length: 12952 diff -r e3c43da3934b gdb/defs.h --- a/gdb/defs.h Tue Oct 07 11:21:29 2008 -0400 +++ b/gdb/defs.h Thu Oct 09 08:55:57 2008 -0400 @@ -194,6 +194,23 @@ enum language language_minimal, /* All other languages, minimal support only */ nr_languages }; + +/* Parsing an expression is dependent on a certain context such as + the language to use, or the input radix for numbers. This structure + contains that context information. */ + +struct parse_context + { + /* The language to use when parsing an expression. */ + const struct language_defn *language; + + /* The radix to use by default when parsing numbers. */ + int radix; + }; + +/* A function that builds a parse_context structure containing + the current language and the current input radix. */ +struct parse_context current_parse_context (void); enum precision_type { diff -r e3c43da3934b gdb/parse.c --- a/gdb/parse.c Tue Oct 07 11:21:29 2008 -0400 +++ b/gdb/parse.c Thu Oct 09 08:55:57 2008 -0400 @@ -131,6 +131,16 @@ struct funcall }; static struct funcall *funcall_chain; + +struct parse_context +current_parse_context (void) +{ + struct parse_context parse_context; + + parse_context.language = current_language; + parse_context.radix = input_radix; + return parse_context; +} /* Begin counting arguments for a function call, saving the data about any containing call. */ diff -r e3c43da3934b gdb/command.h --- a/gdb/command.h Tue Oct 07 11:21:29 2008 -0400 +++ b/gdb/command.h Thu Oct 09 08:55:57 2008 -0400 @@ -98,34 +98,30 @@ struct cmd_list_element; /* Forward-declarations of the entry-points of cli/cli-decode.c. */ -extern struct cmd_list_element *add_cmd (char *, enum command_class, - void (*fun) (char *, int), char *, - struct cmd_list_element **); +extern struct cmd_list_element * + add_cmd (char *, enum command_class, + void (*fun) (char *, int, struct parse_context), + char *, struct cmd_list_element **); extern struct cmd_list_element *add_alias_cmd (char *, char *, enum command_class, int, struct cmd_list_element **); -extern struct cmd_list_element *add_prefix_cmd (char *, enum command_class, - void (*fun) (char *, int), - char *, - struct cmd_list_element **, - char *, int, - struct cmd_list_element **); +extern struct cmd_list_element * + add_prefix_cmd (char *, enum command_class, + void (*fun) (char *, int, struct parse_context), + char *, struct cmd_list_element **, char *, int, + struct cmd_list_element **); -extern struct cmd_list_element *add_abbrev_prefix_cmd (char *, - enum command_class, - void (*fun) (char *, - int), - char *, - struct cmd_list_element - **, char *, int, - struct cmd_list_element - **); +extern struct cmd_list_element * + add_abbrev_prefix_cmd (char *, enum command_class, + void (*fun) (char *, int, struct parse_context), + char *, struct cmd_list_element **, char *, int, + struct cmd_list_element **); /* Set the commands corresponding callback. */ -typedef void cmd_cfunc_ftype (char *args, int from_tty); +typedef void cmd_cfunc_ftype (char *args, int from_tty, struct parse_context); extern void set_cmd_cfunc (struct cmd_list_element *cmd, cmd_cfunc_ftype *cfunc); @@ -140,7 +136,8 @@ extern void set_cmd_completer (struct cm /* HACK: cagney/2002-02-23: Code, mostly in tracepoints.c, grubs around in cmd objects to test the value of the commands sfunc(). */ extern int cmd_cfunc_eq (struct cmd_list_element *cmd, - void (*cfunc) (char *args, int from_tty)); + void (*cfunc) (char *args, int from_tty, + struct parse_context)); /* Each command object has a local context attached to it. . */ extern void set_cmd_context (struct cmd_list_element *cmd, void *context); @@ -178,14 +175,17 @@ extern int struct cmd_list_element **prefix_cmd, struct cmd_list_element **cmd); -extern struct cmd_list_element *add_com (char *, enum command_class, - void (*fun) (char *, int), char *); +extern struct cmd_list_element * + add_com (char *, enum command_class, + void (*fun) (char *, int, struct parse_context), + char *); extern struct cmd_list_element *add_com_alias (char *, char *, enum command_class, int); -extern struct cmd_list_element *add_info (char *, void (*fun) (char *, int), - char *); +extern struct cmd_list_element * + add_info (char *, void (*fun) (char *, int, struct parse_context), + char *); extern struct cmd_list_element *add_info_alias (char *, char *, int); diff -r e3c43da3934b gdb/cli/cli-decode.h --- a/gdb/cli/cli-decode.h Tue Oct 07 11:21:29 2008 -0400 +++ b/gdb/cli/cli-decode.h Thu Oct 09 08:55:57 2008 -0400 @@ -201,35 +201,32 @@ struct cmd_list_element /* API to the manipulation of command lists. */ -extern struct cmd_list_element *add_cmd (char *, enum command_class, - void (*fun) (char *, int), char *, - struct cmd_list_element **); +extern struct cmd_list_element * + add_cmd (char *, enum command_class, + void (*fun) (char *, int, struct parse_context), + char *, struct cmd_list_element **); extern struct cmd_list_element *add_alias_cmd (char *, char *, enum command_class, int, struct cmd_list_element **); -extern struct cmd_list_element *add_prefix_cmd (char *, enum command_class, - void (*fun) (char *, int), - char *, - struct cmd_list_element **, - char *, int, - struct cmd_list_element **); +extern struct cmd_list_element * + add_prefix_cmd (char *, enum command_class, + void (*fun) (char *, int, struct parse_context), + char *, struct cmd_list_element **, char *, int, + struct cmd_list_element **); -extern struct cmd_list_element *add_abbrev_prefix_cmd (char *, - enum command_class, - void (*fun) (char *, - int), - char *, - struct cmd_list_element - **, char *, int, - struct cmd_list_element - **); +extern struct cmd_list_element * + add_abbrev_prefix_cmd (char *, enum command_class, + void (*fun) (char *, int, struct parse_context), + char *, struct cmd_list_element **, char *, int, + struct cmd_list_element **); /* Set the commands corresponding callback. */ extern void set_cmd_cfunc (struct cmd_list_element *cmd, - void (*cfunc) (char *args, int from_tty)); + void (*cfunc) (char *args, int from_tty, + struct parse_context)); extern void set_cmd_sfunc (struct cmd_list_element *cmd, void (*sfunc) (char *args, int from_tty, @@ -241,7 +238,8 @@ extern void set_cmd_completer (struct cm /* HACK: cagney/2002-02-23: Code, mostly in tracepoints.c, grubs around in cmd objects to test the value of the commands sfunc(). */ extern int cmd_cfunc_eq (struct cmd_list_element *cmd, - void (*cfunc) (char *args, int from_tty)); + void (*cfunc) (char *args, int from_tty, + struct parse_context)); /* Access to the command's local context. */ extern void set_cmd_context (struct cmd_list_element *cmd, void *context); @@ -275,14 +273,17 @@ extern int struct cmd_list_element **prefix_cmd, struct cmd_list_element **cmd); -extern struct cmd_list_element *add_com (char *, enum command_class, - void (*fun) (char *, int), char *); +extern struct cmd_list_element * + add_com (char *, enum command_class, + void (*fun) (char *, int, struct parse_context), + char *); extern struct cmd_list_element *add_com_alias (char *, char *, enum command_class, int); -extern struct cmd_list_element *add_info (char *, void (*fun) (char *, int), - char *); +extern struct cmd_list_element * + add_info (char *, void (*fun) (char *, int, struct parse_context), + char *); extern struct cmd_list_element *add_info_alias (char *, char *, int); diff -r e3c43da3934b gdb/cli/cli-decode.c --- a/gdb/cli/cli-decode.c Tue Oct 07 11:21:29 2008 -0400 +++ b/gdb/cli/cli-decode.c Thu Oct 09 08:55:57 2008 -0400 @@ -57,7 +57,7 @@ static void static void do_cfunc (struct cmd_list_element *c, char *args, int from_tty) { - c->function.cfunc (args, from_tty); /* Ok. */ + c->function.cfunc (args, from_tty, current_parse_context ()); /* Ok. */ } void @@ -88,7 +88,7 @@ set_cmd_sfunc (struct cmd_list_element * int cmd_cfunc_eq (struct cmd_list_element *cmd, - void (*cfunc) (char *args, int from_tty)) + void (*cfunc) (char *args, int from_tty, struct parse_context)) { return cmd->func == do_cfunc && cmd->function.cfunc == cfunc; } @@ -149,7 +149,8 @@ set_cmd_completer (struct cmd_list_eleme of *LIST). */ struct cmd_list_element * -add_cmd (char *name, enum command_class class, void (*fun) (char *, int), +add_cmd (char *name, enum command_class class, + void (*fun) (char *, int, struct parse_context), char *doc, struct cmd_list_element **list) { struct cmd_list_element *c @@ -261,7 +262,8 @@ add_alias_cmd (char *name, char *oldname of the variable containing that list. */ struct cmd_list_element * -add_prefix_cmd (char *name, enum command_class class, void (*fun) (char *, int), +add_prefix_cmd (char *name, enum command_class class, + void (*fun) (char *, int, struct parse_context), char *doc, struct cmd_list_element **prefixlist, char *prefixname, int allow_unknown, struct cmd_list_element **list) @@ -277,7 +279,8 @@ add_prefix_cmd (char *name, enum command struct cmd_list_element * add_abbrev_prefix_cmd (char *name, enum command_class class, - void (*fun) (char *, int), char *doc, + void (*fun) (char *, int, struct parse_context), + char *doc, struct cmd_list_element **prefixlist, char *prefixname, int allow_unknown, struct cmd_list_element **list) { @@ -657,7 +660,8 @@ delete_cmd (char *name, struct cmd_list_ /* Add an element to the list of info subcommands. */ struct cmd_list_element * -add_info (char *name, void (*fun) (char *, int), char *doc) +add_info (char *name, void (*fun) (char *, int, struct parse_context), + char *doc) { return add_cmd (name, no_class, fun, doc, &infolist); } @@ -673,7 +677,8 @@ add_info_alias (char *name, char *oldnam /* Add an element to the list of commands. */ struct cmd_list_element * -add_com (char *name, enum command_class class, void (*fun) (char *, int), +add_com (char *name, enum command_class class, + void (*fun) (char *, int, struct parse_context), char *doc) { return add_cmd (name, class, fun, doc, &cmdlist); diff -r e3c43da3934b gdb/exceptions.h --- a/gdb/exceptions.h Tue Oct 07 11:21:29 2008 -0400 +++ b/gdb/exceptions.h Thu Oct 09 08:55:57 2008 -0400 @@ -233,7 +233,7 @@ extern int catch_errors (catch_errors_ft /* Template to catch_errors() that wraps calls to command functions. */ -typedef void (catch_command_errors_ftype) (char *, int); +typedef void (catch_command_errors_ftype) (char *, int, struct parse_context); extern int catch_command_errors (catch_command_errors_ftype *func, char *command, int from_tty, return_mask); #endif diff -r e3c43da3934b gdb/exceptions.c --- a/gdb/exceptions.c Tue Oct 07 11:21:29 2008 -0400 +++ b/gdb/exceptions.c Thu Oct 09 08:55:57 2008 -0400 @@ -528,7 +528,7 @@ catch_command_errors (catch_command_erro volatile struct gdb_exception e; TRY_CATCH (e, mask) { - command (arg, from_tty); + command (arg, from_tty, current_parse_context ()); } print_any_exception (gdb_stderr, NULL, e); if (e.reason < 0) diff -r e3c43da3934b gdb/language.c --- a/gdb/language.c Tue Oct 07 11:21:29 2008 -0400 +++ b/gdb/language.c Thu Oct 09 08:55:57 2008 -0400 @@ -59,9 +59,9 @@ static void unk_lang_error (char *); static int unk_lang_parser (void); -static void show_check (char *, int); +static void show_check (char *, int, struct parse_context parse_context); -static void set_check (char *, int); +static void set_check (char *, int, struct parse_context parse_context); static void set_type_range_case (void); @@ -917,7 +917,7 @@ language_str (enum language lang) } static void -set_check (char *ignore, int from_tty) +set_check (char *ignore, int from_tty, struct parse_context parse_context) { printf_unfiltered ( "\"set check\" must be followed by the name of a check subcommand.\n"); @@ -925,7 +925,7 @@ set_check (char *ignore, int from_tty) } static void -show_check (char *ignore, int from_tty) +show_check (char *ignore, int from_tty, struct parse_context parse_context) { cmd_show_list (showchecklist, from_tty, ""); } --vkogqOf2sHV7VnPd--