From: Joel Brobecker <brobecker@adacore.com>
To: gdb-patches@sourceware.org
Subject: [RFC/RFA] add struct parse_context to all command functions
Date: Thu, 09 Oct 2008 14:05:00 -0000 [thread overview]
Message-ID: <20081009140424.GD5372@adacore.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 4194 bytes --]
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
[-- Attachment #2: parse_context.diff --]
[-- Type: text/plain, Size: 12952 bytes --]
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, "");
}
next reply other threads:[~2008-10-09 14:05 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-09 14:05 Joel Brobecker [this message]
2008-10-09 16:13 ` Tom Tromey
2008-10-09 22:20 ` Joel Brobecker
2008-10-20 16:16 ` Joel Brobecker
2008-10-20 19:03 ` Thiago Jung Bauermann
2008-10-21 0:47 ` Daniel Jacobowitz
2008-10-21 18:06 ` Ulrich Weigand
2008-10-21 18:18 ` Daniel Jacobowitz
2008-10-22 1:40 ` Joel Brobecker
2008-10-22 20:15 ` Tom Tromey
2008-10-22 20:36 ` Joel Brobecker
2008-10-21 1:22 ` Tom Tromey
2008-10-21 7:07 ` Joel Brobecker
2008-10-21 18:12 ` Ulrich Weigand
2008-10-21 18:58 ` Tom Tromey
2008-10-21 19:33 ` Tom Tromey
2008-10-22 18:03 ` Ulrich Weigand
2008-10-22 18:54 ` Tom Tromey
2008-10-22 22:24 ` Ulrich Weigand
2008-10-23 1:02 ` Tom Tromey
2008-10-23 19:50 ` Ulrich Weigand
2008-10-23 21:29 ` Tom Tromey
2008-10-24 13:01 ` Ulrich Weigand
2008-10-25 16:05 ` Tom Tromey
2008-10-27 17:07 ` Tom Tromey
2008-10-28 16:27 ` Ulrich Weigand
2008-10-28 18:17 ` Tom Tromey
2008-10-28 20:00 ` Ulrich Weigand
2008-10-25 16:25 ` Joel Brobecker
2008-10-25 16:46 ` Tom Tromey
2008-10-26 15:19 ` Joel Brobecker
2008-10-21 18:05 ` Ulrich Weigand
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20081009140424.GD5372@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox