From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 102702 invoked by alias); 18 Apr 2018 16:57:59 -0000 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 Received: (qmail 102688 invoked by uid 89); 18 Apr 2018 16:57:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3 autolearn=ham version=3.3.2 spammy=drill, Wow, wow, sk:complai X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 18 Apr 2018 16:57:57 +0000 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 52895326226E; Wed, 18 Apr 2018 16:57:56 +0000 (UTC) Received: from theo.uglyboxes.com (ovpn04.gateway.prod.ext.phx2.redhat.com [10.5.9.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id F3D6C6B242; Wed, 18 Apr 2018 16:57:55 +0000 (UTC) Subject: Re: [RFA 3/4] Remove TYPE_TAG_NAME To: Tom Tromey , gdb-patches@sourceware.org References: <20180417195125.14200-1-tom@tromey.com> <20180417195125.14200-4-tom@tromey.com> From: Keith Seitz Message-ID: <072a09ed-40a3-4e15-2a67-54ac58d1f82a@redhat.com> Date: Wed, 18 Apr 2018 16:57:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <20180417195125.14200-4-tom@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-04/txt/msg00372.txt.bz2 On 04/17/2018 12:51 PM, Tom Tromey wrote: > ChangeLog > 2018-04-17 Tom Tromey > > * 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) : 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 > > * 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 ) -> 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}" Was the tag name never printed until now? 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