On 08/02/2011 01:36 PM, Jan Kratochvil wrote:
>> +/* Merge the two parse trees given by DEST and SRC.  The parse tree
>> +   in SRC is attached to DEST at the node represented by TARGET.
>> +   SRC is then freed.  */
>
> It would need to state that DEST will then still possibly reference the
> original string of SRC.  Commented more at `struct demangle_parse_info'.

I have updated the comments.

> Why not just:
>    *target = *src->tree;

Changed.

> You can calculate LAST only if it gets used - move the code more down.

That block got moved around based on Tom's comments. And obstacks 
cleaned a lot of it up, too.

> In some cases type == otype here, such as if there was only one typedef of
> anonymous type or of typedef failed to be resolved.  Here could be also just
> return 0.

Done.

> double-free, NAME is already in FREE_LIST.
> LEN is not set here for NAME.  And CANON is leaked.

Yup. Cleaned up with the move to an obstack.

> ui_file_rewind should happen even if N is NULL, somehow probably to rather
> abort the operation on NULL N.

Agreed.

> IMO here is missing:
> 	ret_comp->type = DEMANGLE_COMPONENT_NAME;

Indeed!

> If it returns NULL I would prefer some sort of abort.  It risks now to quietly
> corrupt the name.  The later operations will probably fail not modifying
> anything but still it is a bit fragile.

Done.

>> +	case DEMANGLE_COMPONENT_TYPED_NAME:
>> +	  {
>> +	    struct demangle_component *comp = d_right (ret_comp);
>> +
>> +	    while (comp != NULL
>> +		&&  (comp->type == DEMANGLE_COMPONENT_VOLATILE
>> +		       || comp->type == DEMANGLE_COMPONENT_RESTRICT
>> +		       || comp->type == DEMANGLE_COMPONENT_CONST
>> +		       || comp->type == DEMANGLE_COMPONENT_VOLATILE_THIS
>> +		       || comp->type == DEMANGLE_COMPONENT_RESTRICT_THIS
>> +		       || comp->type == DEMANGLE_COMPONENT_CONST_THIS))
>> +	      comp = d_left (comp);
>> +
>> +	    if (d_left (ret_comp)->type != DEMANGLE_COMPONENT_NAME
>> +		|| comp->type != DEMANGLE_COMPONENT_FUNCTION_TYPE)
>> +	      replace_typedefs (info, d_left (ret_comp), free_list);
>
> I admit I do not understand the goal of the COMP computation and the
> conditional.  replace_typedefs unconditionally does not break
> gdb.cp/meth-typedefs.exp.  At least a comment of the purpose would be nice.

I've removed this to unconditionally call replace_typedefs. IIRC, that 
was needed for some reason that escapes me at the moment. The real need 
for it is being hidden by the minsym fallback. I'll follow-up on this 
after this patchset is finalized. [That entails a lot of fixes to 
c-typeprint.c and whatnot called by dwarf2_physname when physnames are 
computed.]

> You are now using cp_canonicalize_string_no_typedefs at the consumers but it
> is not used at the producer - at dwarf2_canonicalize_name.  It is not needed
> there as long as GDB depends on DW_AT_MIPS_linkage_name.  But do you plan to
> use it at dwarf2_canonicalize_name?  That is it would fix
> the `set debug check-physname yes' warnings.

When physnames are computed, no typedefs should appear in them (although 
apparently exceptions should exist for std::string and others that are 
excepted in libiberty). This is a bunch of bugs which I will address 
immediately after this is finalized.

Thank you for reviewing this (again)!

New patch attached.

Keith

ChangeLog
2011-08-09  Keith Seitz  <keiths@redhat.com>

	* cp-support.h (cp_canonicalize_string_no_typedefs): Declare.
	(cp_merge_demangle_parse_infos): Declare.
	* cp-support.c (ignore_typedefs): New file global.
	(copy_string_to_obstack): New function.
	(inspect_type): New function.
	(replace_typedefs): New function.
	(replace_typedefs_qualified_name): New function.
	(cp_canonicalize_string_no_typedefs): New function.
	* cp-name-parser.y (cp_merge_demangle_parse_infos): New function.
	(cp_new_demangle__parse_info): Allocate and initialize the obstack.
	* linespec.c (find_methods): Use cp_canonicalize_string_no_typedefs
	instead of cp_canonicalize_string.
	(find_method): Likewise.
	(decode_compound): Before looking up the name, call
	cp_canonicalize_string_no_typedefs.
	(decode_variable): Likewise.

2011-08-09  Keith Seitz  <keiths@redhat.com>

	* gdb.cp/meth-typedefs.cc: New file.
	* gdb.cp/meth-typedefs.exp: New file.