Re: [patch 2/2] ptype should list also class's typedefs

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> GDB now fully supports by patch 1/2 class's typedefs so it should
Jan> also print:
Jan> type = class C::OtherFileClass {
Jan>   public:
Jan>     int z;
Jan> [...]
Jan>     C::OtherFileClass::cOtherFileClassType cOtherFileClassVar_use(void);

Jan>     typedef short cOtherFileClassType;
Jan>     typedef long cOtherFileClassType2;
Jan> }
Jan> [ the last two typedef lines are new ]

I agree.

Jan> (B) Another possibility is to suppress creating global symbols for
Jan> such types and always look them up through the types listed at the
Jan> `C' class symbol.

Yeah.

Jan> As we should support even the `namespace' case in those cases there
Jan> must exist global symbols with fully qualified names `C::name' -
Jan> (B) way not always applicable.

If we really wanted multi-level symbol tables, then a namespace could
also contain the symbols defined inside it.

For me the key question is what practical advantage is there of a
multi-level symbol table.  It is a cleaner design, but I'm only really
interested in it if it confers some other benefit.

One hidden benefit of our current system, btw, is that things like
lookups of imported variables work even if they are defined in some
other CU, because the lookups are done by name.  If we had a multi-level
table we would also have to solve a cross-objfile problem here.  (We may
have to solve it anyway ...)

Jan> I did not try to go the (A) way but it will be definitely much less
Jan> performance-wise.  We would have to limit search to the CU
Jan> (Compilation Unit) of the owning `C' class.

I'm not so sure.  In fact due to ODR and GCC omitting things I think we
would need a global search.

Jan> It is also questionable whether new
Jan> TYPE_CPLUS_SPECIFIC(type)->typedef_field list should be used as
Jan> implemented by the patch or whether TYPE_FIELDS should be used
Jan> instead.  I am for removing TYPE_NFIELDS completely in the longterm
Jan> and provide all the items in union elements specific for very each
Jan> type_code.  Overloading of TYPE_FIELDS for each type_code is
Jan> currently very hard to read.

I think your approach is fine.

It is customary to make new wrapper macros for new fields here.
I'm not sure that adds much benefit, but maybe just consistency is a
good enough reason.

Jan> It also means the order of fields and typedefs is not preserved,
Jan> though.  (Fields order is preserved and typedefs order is preserved
Jan> but they are kept as two separate sequences.)  I do not find it as
Jan> a problem but even in such case I would propose introducing some
Jan> new index at each stored field instead of overloading TYPE_FIELDS
Jan> again.

I'm not super concerned about the relative ordering of fields and
typedefs.

Access permissions are also dropped, that seems maybe a little more
useful.

I don't think we have to go completely overboard since usually the user
has the source anyhow.

Jan> There should be also a new MI interface, I haven't checked it more
Jan> so far.

ISTR that MI is weak for ptype anyhow.

Jan> +  gdb_assert (die->tag == DW_TAG_typedef);
Jan> +
Jan> +  fp = &new_field->field;
Jan> +
Jan> +  /* Get name of field.  */
Jan> +  fp->name = dwarf2_name (die, cu);
Jan> +  if (fp->name == NULL)
Jan> +    return;
Jan> +
Jan> +  fp->type = die_type (die, cu);

This seems a little odd.  Why not just make a new typedef type, and
store that directly in the outer type?  It seems like that would save a
little space, but more importantly give "C::t" the correct type -- the
typedef and not the underlying type.  (We aren't typedef-correct right
now, but want to be...)

Jan> +	/* Unqualified name to be prefixed by owning class qualified name.  */
Jan> +	const char *name;

I guess with the above approach you'd have to strip the qualifiers
before printing the typedef name.  But, it seems like that should be
simple, since the qualifier is just the enclosing class' name.

Tom