Re: [PATCH 6/7] Move language stuff out of varobj.c

[Joel Brobecker <brobecker@adacore.com>]

Sorry for the delay in reviewing this...

> The new patch 4/7 affects this patch.  I post the updated one.  Patch
> 7/7 is not affected, so it can be reviewed too.
> 
> -- 
> Yao (齐尧)
> 
> gdb:
> 
> 2013-10-11  Yao Qi  <yao@codesourcery.com>
> 
> 	* Makefile.in (SFILES): Add c-varobj.c and jv-varobj.c.
> 	(COMMON_OBS): Add c-varobj.o and jv-varobj.o.
> 	* ada-varobj.c: Include "varobj.h".
> 	(ada_number_of_children): New.  Moved from varobj.c.
> 	(ada_name_of_variable, ada_name_of_child): Likewise.
> 	(ada_path_expr_of_child, ada_value_of_child): Likewise.
> 	(ada_type_of_child, ada_value_of_variable): Likewise.
> 	(ada_value_is_changeable_p, ada_value_has_mutated): Likewise.
> 	(ada_varobj_ops): New.
> 	* c-varobj.c, jv-varobj.c: New file.  Moved from varobj.c.
> 	* gdbtypes.c (get_target_type): New.  Moved from varobj.c.
> 	* gdbtypes.h (get_target_type): Declare.
> 	* varobj.c: Remove the inclusion of "ada-varobj.h" and
> 	"ada-lang.h".
> 	(ANONYMOUS_STRUCT_NAME): Move it to c-varobj.c.
> 	(ANONYMOUS_UNION_NAME): Likewise.
> 	(get_type, get_value_type, get_target_type): Remove declarations.
> 	(value_get_print_value, varobj_value_get_print_value): Likewise.
> 	(c_number_of_children, c_name_of_variable): Likewise.
> 	(c_name_of_child, c_path_expr_of_child): Likewise.
> 	(c_value_of_child, c_type_of_child): Likewise.
> 	(c_value_of_variable, cplus_number_of_children): Likewise.
> 	(cplus_class_num_children, cplus_name_of_variable): Likewise.
> 	(cplus_name_of_child, cplus_path_expr_of_child): Likewise.
> 	(cplus_value_of_child, cplus_type_of_child): Likewise.
> 	(cplus_value_of_variable, java_number_of_children): Likewise.
> 	(java_name_of_variable, java_name_of_child): Likewise.
> 	(java_path_expr_of_child, java_value_of_child): Likewise.
> 	(java_type_of_child, java_value_of_variable): Likewise.
> 	(ada_number_of_children, ada_name_of_variable): Likewise.
> 	(ada_name_of_child, ada_path_expr_of_child): Likewise.
> 	(ada_value_of_child, ada_type_of_child): Likewise.
> 	(ada_value_of_variable, ada_value_is_changeable_p): Likewise.
> 	(ada_value_has_mutated): Likewise.
> 	(struct language_specific): Move it to varobj.h.
> 	(CPLUS_FAKE_CHILD): Move it to varobj.h.
> 	(restrict_range): Rename it varobj_restrict_range.  Make it extern.
> 	Callers update.
> 	(get_path_expr_parent): Rename it to varobj_get_path_expr_parent.
> 	Make it extern.
> 	(is_anonymous_child): Move it to c-varobj.c and rename to
> 	varobj_is_anonymous_child.  Caller update.
> 	(get_type): Move it to c-varobj.c.
> 	(get_value_type): Rename it varobj_get_value_type.  Make it
> 	extern.
> 	(get_target_type): Move it gdbtypes.c.
> 	(varobj_formatted_print_options): New function.
> 	(value_get_print_value): Rename it to
> 	varobj_value_get_print_value and make it extern.
> 	(varobj_value_is_changeable_p): Make it extern.
> 	(adjust_value_for_child_access): Move it to c-varobj.c.
> 	(default_value_is_changeable_p): Rename it to
> 	varobj_default_value_is_changeable_p.  Make it extern.
> 	(c_number_of_children, c_name_of_variable): Move it to c-varobj.c
> 	(c_name_of_child, c_path_expr_of_child): Likewise.
> 	(c_value_of_child, c_type_of_child): Likewise.
> 	(c_value_of_variable, cplus_number_of_children): Likewise.
> 	(cplus_class_num_children, cplus_name_of_variable): Likewise.
> 	(cplus_name_of_child, cplus_path_expr_of_child): Likewise.
> 	(cplus_value_of_child, cplus_type_of_child): Likewise.
> 	(cplus_value_of_variable): Likewise.
> 	(java_number_of_children, java_name_of_variable): Move it to jv-varobj.c.
> 	(java_name_of_child, java_path_expr_of_child): Likewise.
> 	(java_value_of_child, java_type_of_child): Likewise.
> 	(java_value_of_variable): Likewise.
> 	(ada_number_of_children, ada_name_of_variable): Move it to ada-varobj.c.
> 	(ada_name_of_child, ada_path_expr_of_child): Likewise.
> 	(ada_value_of_child, ada_type_of_child): Likewise.
> 	(ada_value_of_variable, ada_value_is_changeable_p): Likewise.
> 	(ada_value_has_mutated): Likewise.
> 	* varobj.h (CPLUS_FAKE_CHILD): New macro, moved from varobj.c.
> 	(struct lang_varobj_ops): New.  Renamed by 'struct language_specific'.
> 	(c_varobj_ops, cplus_varobj_ops): Declare.
> 	(java_varobj_ops, ada_varobj_ops): Declare.
> 	(varobj_default_value_is_changeable_p): Declare.
> 	(varobj_value_is_changeable_p): Declare.
> 	(varobj_get_value_type, varobj_is_anonymous_child): Declare.
> 	(varobj_get_path_expr_parent): Declare.
> 	(varobj_value_get_print_value): Declare.
> 	(varobj_formatted_print_options): Declare.
> 	(varobj_restrict_range): Declare.

Overall, this looks pretty good to me. Just a few nits.

> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 6c809a4..4da6ca2 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -239,6 +239,22 @@ get_type_arch (const struct type *type)
>      return TYPE_OWNER (type).gdbarch;
>  }
>  
> +/* This returns the target type (or NULL) of TYPE, also skipping
> +   past typedefs.  */
> +
> +struct type *
> +get_target_type (struct type *type)

I noticed you documented this function twice, once in the header,
and ones in the body. Can you you only document it once? You have
two options: Remove the documentation in the header entirely, or
replace the documentation in the body by "See gdbtype.h" or some
such. I can't remember exactly the style that TomT introduced for
that - perhaps seem minsyms.h, IIRC.

> -  /* Language info for this variable and its children.  */
> -  struct language_specific *lang;
> +  /* Language-related operations for this variable and its
> +     children.  */
> +  struct lang_varobj_ops *lang;

Please make that field a "const". That way...

> @@ -632,7 +434,7 @@ varobj_create (char *objname,
>  
>        /* Set language info */
>        lang = variable_language (var);
> -      var->root->lang = &languages[lang];
> +      var->root->lang = (struct lang_varobj_ops *) languages[lang];

... we can get rid of the cast here.

Approved with those changes.

Thanks,
-- 
Joel