[RFA 3/4] Remove TYPE_TAG_NAME
Keith Seitz
keiths@redhat.com
Wed Apr 18 16:57:00 GMT 2018
On 04/17/2018 12:51 PM, Tom Tromey wrote:
> ChangeLog
> 2018-04-17 Tom Tromey <tom@tromey.com>
>
> * valops.c (enum_constant_from_type, value_namespace_elt)
> (value_maybe_namespace_elt): Update.
> * valarith.c (find_size_for_pointer_math): Update.
> * target-descriptions.c (make_gdb_type): Update.
> * symmisc.c (print_symbol): Update.
> * stabsread.c (define_symbol, read_type)
> (complain_about_struct_wipeout, add_undefined_type)
> (cleanup_undefined_types_1): Update.
> * rust-lang.c (rust_tuple_type_p, rust_slice_type_p)
> (rust_range_type_p, val_print_struct, rust_print_struct_def)
> (rust_internal_print_type, rust_composite_type)
> (rust_evaluate_funcall, rust_evaluate_subexp): Update.
> * python/py-type.c (typy_get_tag): Update.
> * p-typeprint.c (pascal_type_print_base): Update.
> * mdebugread.c (parse_symbol, parse_type): Update.
> * m2-typeprint.c (m2_long_set, m2_record_fields, m2_enum):
> Update.
> * guile/scm-type.c (gdbscm_type_tag): Update.
> * go-lang.c (sixg_string_p): Update.
> * gnu-v3-abi.c (build_gdb_vtable_type, build_std_type_info_type):
> Update.
> * gdbtypes.h (struct main_type) <tag_name>: Remove.
> (TYPE_TAG_NAME): Remove.
> * gdbtypes.c (type_name_no_tag): Simplify.
> (check_typedef, check_types_equal, recursive_dump_type)
> (copy_type_recursive, arch_composite_type): Update.
> * f-typeprint.c (f_type_print_base): Update. Print "Type" prefix
> in summary mode when needed.
> * eval.c (evaluate_funcall): Update.
> * dwarf2read.c (fixup_go_packaging, read_structure_type)
> (process_structure_scope, read_enumeration_type)
> (read_namespace_type, read_module_type, determine_prefix): Update.
> * cp-support.c (inspect_type): Update.
> * coffread.c (process_coff_symbol, decode_base_type): Update.
> * c-varobj.c (c_is_path_expr_parent): Update.
> * c-typeprint.c (c_type_print_base_struct_union): Update.
> (c_type_print_base_1): Update. Print struct/class/union/enum in
> summary when using C language.
> * ax-gdb.c (gen_struct_ref, gen_namespace_elt)
> (gen_maybe_namespace_elt): Update.
> * ada-lang.c (ada_type_name): Simplify.
> (empty_record, ada_template_to_fixed_record_type_1)
> (template_to_static_fixed_type)
> (to_record_with_fixed_variant_part, ada_check_typedef): Update.
>
> testsuite/ChangeLog
> 2018-04-17 Tom Tromey <tom@tromey.com>
>
> * gdb.xml/tdesc-regs.exp (load_description): Update expected
> results.
> * gdb.dwarf2/method-ptr.exp: Set language to C++.
> * gdb.dwarf2/member-ptr-forwardref.exp: Set language to C++.
> * gdb.cp/typeid.exp (do_typeid_tests): Update type_re.
> * gdb.base/maint.exp (maint_pass_if): Update.
Oh, ChangeLogs... Once again I wonder if "update all callers" would have saved you quite a bit of busywork?
> diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
> index a53331aefc..846727f298 100644
> --- a/gdb/c-typeprint.c
> +++ b/gdb/c-typeprint.c
> @@ -1602,15 +1602,31 @@ c_type_print_base_1 (struct type *type, struct ui_file *stream,
>
> /* When SHOW is zero or less, and there is a valid type name, then
> always just print the type name directly from the type. */
> - /* If we have "typedef struct foo {. . .} bar;" do we want to print
> - it as "struct foo" or as "bar"? Pick the latter, because C++
> - folk tend to expect things like "class5 *foo" rather than "struct
> - class5 *foo". */
>
> if (show <= 0
> && TYPE_NAME (type) != NULL)
> {
> c_type_print_modifier (type, stream, 0, 1);
> +
> + /* If we have "typedef struct foo {. . .} bar;" do we want to
> + print it as "struct foo" or as "bar"? Pick the latter for
> + C++, because C++ folk tend to expect things like "class5
> + *foo" rather than "struct class5 *foo". */
> + if (language == language_c || language == language_minimal)
> + {
> + if (TYPE_CODE (type) == TYPE_CODE_UNION)
> + fprintf_filtered (stream, "union ");
> + else if (TYPE_CODE (type) == TYPE_CODE_STRUCT)
> + {
> + if (TYPE_DECLARED_CLASS (type))
> + fprintf_filtered (stream, "class ");
> + else
> + fprintf_filtered (stream, "struct ");
> + }
> + else if (TYPE_CODE (type) == TYPE_CODE_ENUM)
> + fprintf_filtered (stream, "enum ");
> + }
> +
> print_name_maybe_canonical (TYPE_NAME (type), flags, stream);
> return;
> }
I'm almost afraid to ask, but why was language_minimal necessary here? A small comment might be appropriate?
[I think I can already guess the heinous reason...] Do you know if there is a test case that specifically covers this block with language_minimal?
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index 552a2a2a16..70dac75855 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -721,26 +721,12 @@ struct main_type
>
> /* * Name of this type, or NULL if none.
>
> - This is used for printing only, except by poorly designed C++
> - code. For looking up a name, look for a symbol in the
> - VAR_DOMAIN. This is generally allocated in the objfile's
> - obstack. However coffread.c uses malloc. */
> + This is used for printing only. For looking up a name, look for
> + a symbol in the VAR_DOMAIN. This is generally allocated in the
> + objfile's obstack. However coffread.c uses malloc. */
Good riddance! I never did understand the "except by poorly designed C++ code" comment.
> diff --git a/gdb/cp-support.c b/gdb/cp-support.c
> index dde417be80..c16b67171f 100644
> --- a/gdb/cp-support.c
> +++ b/gdb/cp-support.c
> @@ -199,7 +199,7 @@ inspect_type (struct demangle_parse_info *info,
> && strcmp (TYPE_NAME (type), name) == 0)
> return 0;
>
> - is_anon = (TYPE_TAG_NAME (type) == NULL
> + is_anon = (TYPE_NAME (type) == NULL
> && (TYPE_CODE (type) == TYPE_CODE_ENUM
> || TYPE_CODE (type) == TYPE_CODE_STRUCT
> || TYPE_CODE (type) == TYPE_CODE_UNION));
> diff --git a/gdb/guile/scm-type.c b/gdb/guile/scm-type.c
> index ce67695b76..949e0dca82 100644
> --- a/gdb/guile/scm-type.c
> +++ b/gdb/guile/scm-type.c
> @@ -576,10 +576,16 @@ gdbscm_type_tag (SCM self)
> type_smob *t_smob
> = tyscm_get_type_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
> struct type *type = t_smob->type;
> + const char *tagname = nullptr;
>
> - if (!TYPE_TAG_NAME (type))
> + if (TYPE_CODE (type) == TYPE_CODE_STRUCT
> + || TYPE_CODE (type) == TYPE_CODE_UNION
> + || TYPE_CODE (type) == TYPE_CODE_ENUM)
> + tagname = TYPE_NAME (type);
> +
> + if (tagname == nullptr)
> return SCM_BOOL_F;
> - return gdbscm_scm_from_c_string (TYPE_TAG_NAME (type));
> + return gdbscm_scm_from_c_string (tagname);
> }
>
> /* (type-name <gdb:type>) -> string
> diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
> index 8a948eeaa6..6cbb1b663d 100644
> --- a/gdb/python/py-type.c
> +++ b/gdb/python/py-type.c
> @@ -416,10 +416,16 @@ static PyObject *
> typy_get_tag (PyObject *self, void *closure)
> {
> struct type *type = ((type_object *) self)->type;
> + const char *tagname = nullptr;
>
> - if (!TYPE_TAG_NAME (type))
> + if (TYPE_CODE (type) == TYPE_CODE_STRUCT
> + || TYPE_CODE (type) == TYPE_CODE_UNION
> + || TYPE_CODE (type) == TYPE_CODE_ENUM)
> + tagname = TYPE_NAME (type);
> +
> + if (tagname == nullptr)
> Py_RETURN_NONE;
> - return PyString_FromString (TYPE_TAG_NAME (type));
> + return PyString_FromString (tagname);
> }
>
> /* Return the type, stripped of typedefs. */
The above three code snippets suggest the introduction of a type_tag_name (struct type *) convenience function might be justified, but I am not going to request any such change.
> diff --git a/gdb/testsuite/gdb.xml/tdesc-regs.exp b/gdb/testsuite/gdb.xml/tdesc-regs.exp
> index bb7b9be16c..94d4007a89 100644
> --- a/gdb/testsuite/gdb.xml/tdesc-regs.exp
> +++ b/gdb/testsuite/gdb.xml/tdesc-regs.exp
> @@ -179,7 +179,7 @@ gdb_test "ptype \$extrareg" "type = (int|long|long long)"
> gdb_test "ptype \$uintreg" "type = uint32_t"
> gdb_test "ptype \$vecreg" "type = int8_t __attribute__ \\(\\(vector_size\\(4\\)\\)\\)"
> gdb_test "ptype \$unionreg" \
> - "type = union {\r\n *v4int8 v4;\r\n *v2int16 v2;\r\n}"
> + "type = union vecint {\r\n *v4int8 v4;\r\n *v2int16 v2;\r\n}"
> gdb_test "ptype \$unionreg.v4" "type = int8_t __attribute__ \\(\\(vector_size\\(4\\)\\)\\)"
> gdb_test "ptype \$structreg" \
> "type = struct struct1 {\r\n *v4int8 v4;\r\n *v2int16 v2;\r\n}"
<rhetorical>Was the tag name never printed until now?</rhetorical> Wow!
> @@ -189,7 +189,7 @@ gdb_test "ptype \$bitfields" \
> gdb_test "ptype \$flags" \
> "type = flag flags {\r\n *bool X @0;\r\n *uint32_t Y @2;\r\n}"
> gdb_test "ptype \$mixed_flags" \
> - "type = flag mixed_flags {\r\n *bool A @0;\r\n *uint32_t B @1-3;\r\n *bool C @4;\r\n *uint32_t D @5;\r\n *uint32_t @6-7;\r\n *enum {yes = 1, no = 0, maybe = 2, so} Z @8-9;\r\n}"
> + "type = flag mixed_flags {\r\n *bool A @0;\r\n *uint32_t B @1-3;\r\n *bool C @4;\r\n *uint32_t D @5;\r\n *uint32_t @6-7;\r\n *enum Z_values {yes = 1, no = 0, maybe = 2, so} Z @8-9;\r\n}"
> # Reggroups should have at least general and the extra foo group
> gdb_test "maintenance print reggroups" \
> " Group\[ \t\]+Type\[ \t\]+\r\n.* general\[ \t\]+user\[ \t\]+\r\n.* foo\[ \t\]+user\[ \t\]+"
Warning: I am not a maintainer, ... You know the rest of that drill. :-)
Keith
More information about the Gdb-patches
mailing list