This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] c++/12266 (again)

On 08/02/2011 09:08 AM, Tom Tromey wrote:
Maybe 'static const char * const'?

Sure. I've never seen this done before, so I just (blindly) followed suit.

Keith> +typedef char *namep;

defs.h already has 'char_ptr' for this purpose.

Thanks. Changed. [I even went looking for that!]

Keith>  +static void
Keith>  +replace_typedefs (struct demangle_parse_info *info,

Indent this line somehow.

Yes, sorry, I missed that one.

Keith>  +	  while (TYPE_CODE (TYPE_TARGET_TYPE (last)) == TYPE_CODE_TYPEDEF)
Keith>  +	    last = TYPE_TARGET_TYPE (last);

It seems like you could construct a test case where the type is a stub
type, so TYPE_TARGET_TYPE == NULL, causing a crash.

I've added a check for this.

Keith> + /* If there is only one typedef for this anonymous type, Keith> + do not substitute it. */

I don't understand this comment.
How does this code check if this anonymous type only had one typedef?

A little re-arranging of the code (pushing the while loop into the is_anon clause) would be less confusing, I think. While that whole block deals with this, that comment is probably best deleted in favor of explanations closer to the actual code dealing with it.

As for determining if there is only one typedef for a type, the idea is to iterate over all the typedefs for the type. If the original type (otype) is the same as the "last" typedef seen (i.e., TYPE_CODE(TYPE_TARGET_TYPE (last)) != TYPE_CODE_TYPEDEF), then there is only one typedef associated with this type (well, at least in this chain of typedefs). meth-typedefs.exp does exactly this (defines typedefs of typedefs).

If there is a better/easier way, I'm all eyes!

Keith> + (void) inspect_type (info, ret_comp, free_list);

Don't cast to void.


Keith>  +      /* Free any memory allocated during typedef replacement.  */
Keith>  +      if (!VEC_empty (namep, free_list))
You don't need the VEC_empty check here; vec.h will do the right thing
if you try to iterate an empty (or NULL) VEC.


Keith>  +  cleanup = make_cleanup (null_cleanup, NULL);
Keith>  +  if (current_language->la_language == language_cplus)
Keith>  +    {
Keith>  +      char *canon = cp_canonicalize_string_no_typedefs (copy);
Keith>  +
Keith>  +      if (canon != NULL)
Keith>  +	{
Keith>  +	  name = canon;
Keith>  +	  make_cleanup (xfree, name);
Keith>  +	}

This change in linespec checks current_language, but the other does not.
Why is that?

I originally had both checking the language, but while testing, uncovered some problems with that. I removed the check around the problematic one but did not/forgot to do so with this one. I have removed it and verified that it does not impact test results. [And besides, there is no reason for this not to work with C anyway!]

Updated patch attached. Thank you for looking at this.


Attachment: cp_canonicalize_no_typedefs-5.patch
Description: Text document

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]