Re: [PATCH 2/2] Remove varobj_language_string, languages and varobj_languages

Joel Brobecker
2013-10-25T04:15:32Z

> This patch does some cleanups, removing some language-related stuff.
> Note that mi_cmd_var_info_expression uses varobj_language_string,
> which is redundant, because we can get language name from
> lang->la_name and capitalize the first letter.

I think this can do for now because it is good enough, but I would
definitely prefer we avoid this type transformation. Instead,
I'd rather we added a new field in struct language_defn that
provides the "natural" name.

> varobj_language_string doesn't have "Ada", which looks like a bug to
> me. With this patch applied, this problem doesn't exist, because the
> language name is got from the same place (field la_name).

Indeed, there was a bug there, and we're lucky (or maybe not!)
it didn't crash the debugger. I like it better also that we're
no longer depending on a parallel array like this anymore. If we
were, I'd probably implement a consistency check similar to what
we have in osabi.

> 2013-10-18 Yao Qi <yao@codesourcery.com>
>
> * mi/mi-cmd-var.c: Include "language.h" and <ctype.h>.
> (mi_cmd_var_info_expression): Get language name from
> language_defn.
> * varobj.c (varobj_language_string): Remove.
> (variable_language): Remove declaration.
> (languages): Remove.
> (varobj_get_language): Change the type of return value.
> (variable_language): Remove.
> * varobj.h (enum varobj_languages): Remove.
> (varobj_language_string): Remove declaration.
> (varobj_get_language): Update declaration.

Personally, I would do the language_defn update first, and then get
rid of the xstrdup/toupper/xfree dance. But I'm open to others'
opinion.

Note that <ctype.h> was already included.

> ---
> gdb/mi/mi-cmd-var.c | 12 ++++++++++--
> gdb/varobj.c | 43 ++-----------------------------------------
> gdb/varobj.h | 12 +-----------
> 3 files changed, 13 insertions(+), 54 deletions(-)
>
> diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
> index 57a2f6b..59202ff 100644
> --- a/gdb/mi/mi-cmd-var.c
> +++ b/gdb/mi/mi-cmd-var.c
> @@ -24,12 +24,14 @@
> #include "ui-out.h"
> #include "mi-out.h"
> #include "varobj.h"
> +#include "language.h"
> #include "value.h"
> #include <ctype.h>
> #include "gdb_string.h"
> #include "mi-getopt.h"
> #include "gdbthread.h"
> #include "mi-parse.h"
> +#include <ctype.h>
>
> extern unsigned int varobjdebug; /* defined in varobj.c. */
>
> @@ -479,8 +481,9 @@ void
> mi_cmd_var_info_expression (char *command, char **argv, int argc)
> {
> struct ui_out *uiout = current_uiout;
> - enum varobj_languages lang;
> + const struct language_defn *lang;
> struct varobj *var;
> + char *name;
>
> if (argc != 1)
> error (_("-var-info-expression: Usage: NAME."));
> @@ -489,9 +492,14 @@ mi_cmd_var_info_expression (char *command, char **argv, int argc)
> var = varobj_get_handle (argv[0]);
>
> lang = varobj_get_language (var);
> + name = xstrdup (lang->la_name);
> + /* Capitalize it. */
> + name[0] = toupper (name[0]);
>
> - ui_out_field_string (uiout, "lang", varobj_language_string[(int) lang]);
> + ui_out_field_string (uiout, "lang", name);
> ui_out_field_string (uiout, "exp", varobj_get_expression (var));
> +
> + xfree (name);
> }
>
> void
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 60ed810..8989aae 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -55,9 +55,6 @@ show_varobjdebug (struct ui_file *file, int from_tty,
> char *varobj_format_string[] =
> { "natural", "binary", "decimal", "hexadecimal", "octal" };
>
> -/* String representations of gdb's known languages. */
> -char *varobj_language_string[] = { "C", "C++", "Java" };
> -
> /* True if we want to allow Python-based pretty-printing. */
> static int pretty_printing = 0;
>
> @@ -199,8 +196,6 @@ static int install_new_value (struct varobj *var, struct value *value,
>
> /* Language-specific routines. */
>
> -static enum varobj_languages variable_language (struct varobj *var);
> -
> static int number_of_children (struct varobj *);
>
> static char *name_of_variable (struct varobj *);
> @@ -224,14 +219,6 @@ static struct varobj *varobj_add_child (struct varobj *var,
>
> #endif /* HAVE_PYTHON */
>
> -/* Array of known source language routines. */
> -static const struct lang_varobj_ops *languages[vlang_end] = {
> - &c_varobj_ops,
> - &cplus_varobj_ops,
> - &java_varobj_ops,
> - &ada_varobj_ops,
> -};
> -
> /* Private data */
>
> /* Mappings of varobj_display_formats enums to gdb's format codes. */
> @@ -1126,10 +1113,10 @@ varobj_get_path_expr (struct varobj *var)
> }
> }
>
> -enum varobj_languages
> +const struct language_defn *
> varobj_get_language (struct varobj *var)
> {
> - return variable_language (var);
> + return var->root->exp->language_defn;
> }
>
> int
> @@ -2332,32 +2319,6 @@ cppop (struct cpstack **pstack)
>
> /* Common entry points */
>
> -/* Get the language of variable VAR. */
> -static enum varobj_languages
> -variable_language (struct varobj *var)
> -{
> - enum varobj_languages lang;
> -
> - switch (var->root->exp->language_defn->la_language)
> - {
> - default:
> - case language_c:
> - lang = vlang_c;
> - break;
> - case language_cplus:
> - lang = vlang_cplus;
> - break;
> - case language_java:
> - lang = vlang_java;
> - break;
> - case language_ada:
> - lang = vlang_ada;
> - break;
> - }
> -
> - return lang;
> -}
> -
> /* Return the number of children for a given variable.
> The result of this function is defined by the language
> implementation. The number of children returned by this function
> diff --git a/gdb/varobj.h b/gdb/varobj.h
> index f56810c..1748433 100644
> --- a/gdb/varobj.h
> +++ b/gdb/varobj.h
> @@ -52,16 +52,6 @@ enum varobj_scope_status
> /* String representations of gdb's format codes (defined in varobj.c). */
> extern char *varobj_format_string[];
>
> -/* Languages supported by this variable objects system. This enum is used
> - to index arrays so we make its first enum explicitly zero. */
> -enum varobj_languages
> - {
> - vlang_c = 0, vlang_cplus, vlang_java, vlang_ada, vlang_end
> - };
> -
> -/* String representations of gdb's known languages (defined in varobj.c). */
> -extern char *varobj_language_string[];
> -
> /* Struct thar describes a variable object instance. */
>
> struct varobj;
> @@ -286,7 +276,7 @@ extern struct type *varobj_get_gdb_type (struct varobj *var);
>
> extern char *varobj_get_path_expr (struct varobj *var);
>
> -extern enum varobj_languages varobj_get_language (struct varobj *var);
> +extern const struct language_defn *varobj_get_language (struct varobj *var);
>
> extern int varobj_get_attributes (struct varobj *var);
>
> --
> 1.7.7.6

--
Joel