[RFA] Change to pre-expand symtabs

[RFA] Change to pre-expand symtabs

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 1000 bytes --]

Hi,

This patch actually does two things, both related to PR symtab/11743 
(for which I've previously submitted various ugly patches). First, it 
switches gdb to search for symtabs by using psymtab pre-expansion.

Second, it strips off overload information from psymtab search names, 
since psymtabs do not contain any of this information to begin with. I 
believe this is going to be the cleanest approach to fixing the 
overload-instance-in-a-psymtab problem (symtab/11743) that we're likely 
to find.

Note that I have not addressed the linespec.c problems specifically 
reported in symtab/11743. This simply gets the non-quoted case working 
(which is a pre-requisite anyway).

Keith

ChangeLog
2010-07-26  Keith Seitz  <keiths@redhat.com>

	PR symtab/11743 (partial):
	* psymtab.c (lookup_symbol_aux_psymtabs): Always return NULL;
	switching to pre-expanding symtabs instead.
	(pre_expand_symtabs_matching_psymtabs): Implement.
	If overload information is present in the search name,
	strip it.

[-- Attachment #2: expand-psymtabs-overloaded.patch --]
[-- Type: text/plain, Size: 1526 bytes --]

Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.7
diff -u -p -r1.7 psymtab.c
--- psymtab.c	13 Jul 2010 20:52:52 -0000	1.7
+++ psymtab.c	26 Jul 2010 18:38:24 -0000
@@ -409,15 +409,6 @@ lookup_symbol_aux_psymtabs (struct objfi
 			    int block_index, const char *name,
 			    const domain_enum domain)
 {
-  struct partial_symtab *ps;
-  const int psymtab_index = (block_index == GLOBAL_BLOCK ? 1 : 0);
-
-  ALL_OBJFILE_PSYMTABS (objfile, ps)
-  {
-    if (!ps->readin && lookup_partial_symbol (ps, name, psymtab_index, domain))
-      return PSYMTAB_TO_SYMTAB (ps);
-  }
-
   return NULL;
 }
 
@@ -426,7 +417,27 @@ pre_expand_symtabs_matching_psymtabs (st
 				      int kind, const char *name,
 				      domain_enum domain)
 {
-  /* Nothing.  */
+  char *paren;
+  struct partial_symtab *pst;
+  const int psymtab_index = (kind == GLOBAL_BLOCK ? 1 : 0);
+
+  /* If NAME contains overload information, strip it, since psymtabs only
+     contain the method name.  */
+  paren = strchr (name, '(');
+  if (paren != NULL)
+    {
+      char *tmp = alloca (strlen (name));
+      memcpy (tmp, name, paren - name);
+      tmp[name - paren] = '\0';
+      name = tmp;
+    }
+
+  ALL_OBJFILE_PSYMTABS (objfile, pst)
+    {
+      if (!pst->readin
+	  && lookup_partial_symbol (pst, name, psymtab_index, domain) != NULL)
+	PSYMTAB_TO_SYMTAB (pst);
+    }
 }
 
 /* Look, in partial_symtab PST, for symbol whose natural name is NAME.

Re: [RFA] Change to pre-expand symtabs

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> This patch actually does two things, both related to PR symtab/11743
Keith> (for which I've previously submitted various ugly patches). First, it
Keith> switches gdb to search for symtabs by using psymtab pre-expansion.

Thanks.

Would you mind letting me take over this one?  I found a case where
pre-expansion with an index expands way too many symbol tables.  Fixing
this will necessitate changing the pre-expansion API, plus maybe some
other things.

I think it would be simpler for me to fix up the index code first, then
update this patch to fit.  Also I would be able to test this patch for
the over-eager expansion problem as well.

I'm out next week but I will finish up this work the week after.

Tom

Re: [RFA] Change to pre-expand symtabs

[Keith Seitz]

On 07/29/2010 01:52 PM, Tom Tromey wrote:

> Would you mind letting me take over this one?

That's certainly not a problem. Thank you for the offer.

Keith

Re: [RFA] Change to pre-expand symtabs

[Tom Tromey]

Keith> This patch actually does two things, both related to PR symtab/11743
Keith> (for which I've previously submitted various ugly patches). First, it
Keith> switches gdb to search for symtabs by using psymtab pre-expansion.
[...]

Tom> Would you mind letting me take over this one?  I found a case where
Tom> pre-expansion with an index expands way too many symbol tables.

I'm afraid I am going to kick this one back to you.

As we discussed on irc, it has problems if the searched-for name
includes a paren that is not used to enclose the function arguments.

I found one other little buglet:

+      char *tmp = alloca (strlen (name));
+      memcpy (tmp, name, paren - name);
+      tmp[name - paren] = '\0';

That last line should read "paren - name".  (Also it is more efficient
not to use paren-name+1, not strlen, to size the temporary array.)

I've appended a patch which is this patch, updated with that fix, and
updated for the pre-expansion change I'm going to send soon.  This
modified patch causes a regression in type-opaque.exp, I didn't research
why.

Tom

diff --git b/gdb/psymtab.c a/gdb/psymtab.c
index 44ccb0f..54057b2 100644
--- b/gdb/psymtab.c
+++ a/gdb/psymtab.c
@@ -409,15 +409,6 @@ lookup_symbol_aux_psymtabs (struct objfile *objfile,
 			    int block_index, const char *name,
 			    const domain_enum domain)
 {
-  struct partial_symtab *ps;
-  const int psymtab_index = (block_index == GLOBAL_BLOCK ? 1 : 0);
-
-  ALL_OBJFILE_PSYMTABS (objfile, ps)
-  {
-    if (!ps->readin && lookup_partial_symbol (ps, name, psymtab_index, domain))
-      return PSYMTAB_TO_SYMTAB (ps);
-  }
-
   return NULL;
 }
 
@@ -432,6 +423,35 @@ expand_one_symtab_matching_psymtabs (struct objfile *objfile,
 								void *),
 				     void *data)
 {
+  char *paren;
+  struct partial_symtab *pst;
+  const int psymtab_index = (kind == GLOBAL_BLOCK ? 1 : 0);
+
+  /* If NAME contains overload information, strip it, since psymtabs only
+     contain the method name.  */
+  paren = strchr (name, '(');
+  if (paren != NULL)
+    {
+      char *tmp = alloca (paren - name + 1);
+      memcpy (tmp, name, paren - name);
+      tmp[paren - name] = '\0';
+      name = tmp;
+    }
+
+  ALL_OBJFILE_PSYMTABS (objfile, pst)
+    {
+      if (!pst->readin
+	  && lookup_partial_symbol (pst, name, psymtab_index, domain) != NULL)
+	{
+	  struct symtab *symtab = PSYMTAB_TO_SYMTAB (pst);
+	  struct symbol *sym;
+
+	  sym = matcher (symtab, kind, name, domain, data);
+	  if (sym)
+	    return sym;
+	}
+    }
+
   return NULL;
 }
 

Re: [RFA] Change to pre-expand symtabs

[Keith Seitz]

On 09/01/2010 02:32 PM, Tom Tromey wrote:
> I'm afraid I am going to kick this one back to you.

I'll address this in a follow-up. I wanted to address the regression you 
found:

> This modified patch causes a regression in type-opaque.exp, I didn't research
> why.

This is happening because match_transparent_type in symtab.c is assuming 
GLOBAL_BLOCK, even though the desired block type is passed in.

With the simple patch below, the regression is gone.

May I commit this? [It causes no regressions on x86_64 linux.]

Keith

ChangeLog
2010-09-02  Keith Seitz  <keiths@redhat.com>

	* symtab.c (match_transparent_type): Use KIND for determining
	the appropriate BLOCKVECTOR.


Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.252
diff -u -p -r1.252 symtab.c
--- symtab.c	1 Sep 2010 21:50:26 -0000	1.252
+++ symtab.c	2 Sep 2010 16:36:58 -0000
@@ -1671,7 +1671,7 @@ match_transparent_type (struct symtab *s
        struct symbol *sym;

        bv = BLOCKVECTOR (symtab);
-      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
+      block = BLOCKVECTOR_BLOCK (bv, kind);
        sym = lookup_block_symbol (block, name, STRUCT_DOMAIN);
        if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym)))
  	return sym;

Re: [RFA] Change to pre-expand symtabs

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

On 09/01/2010 02:32 PM, Tom Tromey wrote:
> I'm afraid I am going to kick this one back to you.
>
> As we discussed on irc, it has problems if the searched-for name
> includes a paren that is not used to enclose the function arguments.

Here is a revised version of this patch which should eliminate this 
problem completely. If the search name contains a '(' (for C++/Java), we 
pass it to cp_remove_params to strip that information. I tested this 
with your example, and it parses that properly.

[Reminder: This is only the part of the patch requiring changes. The 
testsuite patch is still in play. This also requires my earlier patch 
for match_transparent_type to be regression-free.]

Tested on x86_64-linux: no regressions.

Comments?

Keith

ChangeLog
2010-09-02  Keith Seitz  <keiths@redhat.com>

	PR symtab/11743 (partial):
	* psymtab.c (lookup_symbol_aux_psymtabs): Always return NULL;
	switching to pre-expanding symtabs instead.
	(expand_one_symtab_matching_psymtab): Implement.
	* symtab.c (psymtab_search_name): New function.
	(lookup_symbol_aux_symtabs): Use psymtab_search_name for
	psymtab searches.
	(basic_lookup_transparent_type): Likewise.

[-- Attachment #2: expand-psymtabs-overloaded-2.patch --]
[-- Type: text/plain, Size: 3937 bytes --]

Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.12
diff -u -p -r1.12 psymtab.c
--- psymtab.c	1 Sep 2010 21:50:26 -0000	1.12
+++ psymtab.c	2 Sep 2010 19:25:24 -0000
@@ -414,15 +414,6 @@ lookup_symbol_aux_psymtabs (struct objfi
 			    int block_index, const char *name,
 			    const domain_enum domain)
 {
-  struct partial_symtab *ps;
-  const int psymtab_index = (block_index == GLOBAL_BLOCK ? 1 : 0);
-
-  ALL_OBJFILE_PSYMTABS (objfile, ps)
-  {
-    if (!ps->readin && lookup_partial_symbol (ps, name, psymtab_index, domain))
-      return PSYMTAB_TO_SYMTAB (ps);
-  }
-
   return NULL;
 }
 
@@ -437,6 +428,23 @@ expand_one_symtab_matching_psymtabs (str
 								void *),
 				     void *data)
 {
+  struct partial_symtab *pst;
+  const int psymtab_index = (kind == GLOBAL_BLOCK ? 1 : 0);
+
+  ALL_OBJFILE_PSYMTABS (objfile, pst)
+    {
+      if (!pst->readin
+	  && lookup_partial_symbol (pst, name, psymtab_index, domain) != NULL)
+	{
+	  struct symtab *symtab = PSYMTAB_TO_SYMTAB (pst);
+	  struct symbol *sym;
+
+	  sym = matcher (symtab, kind, name, domain, data);
+	  if (sym)
+	    return sym;
+	}
+    }
+
   return NULL;
 }
 
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.252
diff -u -p -r1.252 symtab.c
--- symtab.c	1 Sep 2010 21:50:26 -0000	1.252
+++ symtab.c	2 Sep 2010 19:25:25 -0000
@@ -759,6 +759,40 @@ symbol_search_name (const struct general
     return symbol_natural_name (gsymbol);
 }
 
+/* Returns the name used to search psymtabs, which do not, for example,
+   contain any overload information. If the user is searching for a
+   specific instance of an overloaded method, it is necessary
+   to strip this information away before conducting the search.  */
+
+static const char *
+psymtab_search_name (const char *name)
+{
+  switch (current_language->la_language)
+    {
+    case language_cplus:
+    case language_java:
+      {
+	static char *ret = NULL;
+
+	if (ret != NULL)
+	  {
+	    xfree (ret);
+	    ret = NULL;
+	  }
+
+	if (strchr (name, '('))
+	  ret = cp_remove_params (name);
+
+	return (ret == NULL) ? name : ret;
+      }
+
+    default:
+      break;
+    }
+
+  return name;
+}
+
 /* Initialize the structure fields to zero values.  */
 void
 init_sal (struct symtab_and_line *sal)
@@ -1427,9 +1461,11 @@ lookup_symbol_aux_symtabs (int block_ind
 
     if (objfile->sf)
       {
+	const char *psym_search_name = psymtab_search_name (name);
 	sym = objfile->sf->qf->expand_one_symtab_matching (objfile,
 							   block_index,
-							   name, domain,
+							   psym_search_name,
+							   domain,
 							   match_symbol_aux,
 							   objfile);
 	if (sym)
@@ -1695,12 +1731,14 @@ basic_lookup_transparent_type (const cha
   struct objfile *objfile;
   struct block *block;
   struct type *t;
+  const char *psym_search_name;
 
   /* Now search all the global symbols.  Do the symtab's first, then
      check the psymtab's. If a psymtab indicates the existence
      of the desired name as a global, then do psymtab-to-symtab
      conversion on the fly and return the found symbol.  */
 
+  psym_search_name = psymtab_search_name (name);
   ALL_OBJFILES (objfile)
   {
     ALL_OBJFILE_SYMTABS (objfile, s)
@@ -1719,7 +1757,8 @@ basic_lookup_transparent_type (const cha
       {
 	sym
 	  = objfile->sf->qf->expand_one_symtab_matching (objfile,
-							 GLOBAL_BLOCK, name,
+							 GLOBAL_BLOCK,
+							 psym_search_name,
 							 STRUCT_DOMAIN,
 							 match_transparent_type,
 							 NULL);
@@ -1760,7 +1799,8 @@ basic_lookup_transparent_type (const cha
       {
 	sym
 	  = objfile->sf->qf->expand_one_symtab_matching (objfile,
-							 STATIC_BLOCK, name,
+							 STATIC_BLOCK,
+							 psym_search_name,
 							 STRUCT_DOMAIN,
 							 match_transparent_type,
 							 NULL);

Re: [RFA] Change to pre-expand symtabs

[Jan Kratochvil]

On Thu, 02 Sep 2010 21:35:04 +0200, Keith Seitz wrote:
> --- symtab.c	1 Sep 2010 21:50:26 -0000	1.252
> +++ symtab.c	2 Sep 2010 19:25:25 -0000
> +psymtab_search_name (const char *name)
> +      {
> +	static char *ret = NULL;
> +
> +	if (ret != NULL)
> +	  {
> +	    xfree (ret);
> +	    ret = NULL;
> +	  }
> +
> +	if (strchr (name, '('))
> +	  ret = cp_remove_params (name);
> +
> +	return (ret == NULL) ? name : ret;
> +      }
[...]
> +	const char *psym_search_name = psymtab_search_name (name);
>  	sym = objfile->sf->qf->expand_one_symtab_matching (objfile,
>  							   block_index,
> -							   name, domain,
> +							   psym_search_name,
> +							   domain,
>  							   match_symbol_aux,
>  							   objfile);

I find such static buffer as needlessly dangerous, in general, without
thinking too much more.

I do not see obviously why some the callee expand_one_symtab_matching could
not recursively call this function (incl. psymtab_search_name again) while
still accessing also its passed parameter, which gets freed this way.

I would just say the caller should xfree the string.  (Yes, it is a pain to do
make_cleanup if appropriate but that part is a different problem.)


Thanks,
Jan

Re: [RFA] Change to pre-expand symtabs

[Keith Seitz]

[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

On 09/04/2010 06:44 AM, Jan Kratochvil wrote:
> I find such static buffer as needlessly dangerous, in general, without
> thinking too much more.

Actually, I think I rushed this patch out. Tom and I were chatting on 
IRC. In the context of the patch I was developing then, this was a 
feasible solution.

However, at Tom's suggestion, I moved some things around, and completely 
forgot to move the memory allocation around with it. My bad.

Good catch, Jan.

I've attached an updated version of this.

Thank you for looking,

Keith

[-- Attachment #2: expand-psymtabs-overloaded-3.patch --]
[-- Type: text/plain, Size: 6412 bytes --]

Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.12
diff -u -p -r1.12 psymtab.c
--- psymtab.c	1 Sep 2010 21:50:26 -0000	1.12
+++ psymtab.c	7 Sep 2010 20:59:54 -0000
@@ -414,15 +414,6 @@ lookup_symbol_aux_psymtabs (struct objfi
 			    int block_index, const char *name,
 			    const domain_enum domain)
 {
-  struct partial_symtab *ps;
-  const int psymtab_index = (block_index == GLOBAL_BLOCK ? 1 : 0);
-
-  ALL_OBJFILE_PSYMTABS (objfile, ps)
-  {
-    if (!ps->readin && lookup_partial_symbol (ps, name, psymtab_index, domain))
-      return PSYMTAB_TO_SYMTAB (ps);
-  }
-
   return NULL;
 }
 
@@ -437,6 +428,23 @@ expand_one_symtab_matching_psymtabs (str
 								void *),
 				     void *data)
 {
+  struct partial_symtab *pst;
+  const int psymtab_index = (kind == GLOBAL_BLOCK ? 1 : 0);
+
+  ALL_OBJFILE_PSYMTABS (objfile, pst)
+    {
+      if (!pst->readin
+	  && lookup_partial_symbol (pst, name, psymtab_index, domain) != NULL)
+	{
+	  struct symtab *symtab = PSYMTAB_TO_SYMTAB (pst);
+	  struct symbol *sym;
+
+	  sym = matcher (symtab, kind, name, domain, data);
+	  if (sym)
+	    return sym;
+	}
+    }
+
   return NULL;
 }
 
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.252
diff -u -p -r1.252 symtab.c
--- symtab.c	1 Sep 2010 21:50:26 -0000	1.252
+++ symtab.c	7 Sep 2010 20:59:55 -0000
@@ -759,6 +759,32 @@ symbol_search_name (const struct general
     return symbol_natural_name (gsymbol);
 }
 
+/* Returns the name used to search psymtabs.  The caller is responsible for
+   freeing the return result.  */
+
+static const char *
+psymtab_search_name (const char *name)
+{
+  switch (current_language->la_language)
+    {
+    case language_cplus:
+    case language_java:
+      {
+	if (strchr (name, '('))
+	  {
+	    char *ret = cp_remove_params (name);
+	    if (ret)
+	      return ret;
+	  }
+      }
+
+    default:
+      break;
+    }
+
+  return xstrdup (name);
+}
+
 /* Initialize the structure fields to zero values.  */
 void
 init_sal (struct symtab_and_line *sal)
@@ -1409,7 +1435,13 @@ lookup_symbol_aux_symtabs (int block_ind
   struct blockvector *bv;
   const struct block *block;
   struct symtab *s;
+  const char *psym_search_name;
+  struct cleanup *cleanup;
+
+  psym_search_name = psymtab_search_name (name);
+  cleanup = make_cleanup (xfree, (void *) psym_search_name);
 
+  sym = NULL;
   ALL_OBJFILES (objfile)
   {
     ALL_OBJFILE_SYMTABS (objfile, s)
@@ -1421,7 +1453,8 @@ lookup_symbol_aux_symtabs (int block_ind
 	  if (sym)
 	    {
 	      block_found = block;
-	      return fixup_symbol_section (sym, objfile);
+	      sym = fixup_symbol_section (sym, objfile);
+	      goto done;
 	    }
 	}
 
@@ -1429,15 +1462,18 @@ lookup_symbol_aux_symtabs (int block_ind
       {
 	sym = objfile->sf->qf->expand_one_symtab_matching (objfile,
 							   block_index,
-							   name, domain,
+							   psym_search_name,
+							   domain,
 							   match_symbol_aux,
 							   objfile);
 	if (sym)
-	  return sym;
+	  goto done;
       }
   }
 
-  return NULL;
+ done:
+  do_cleanups (cleanup);
+  return sym;
 }
 
 /* A helper function for lookup_symbol_aux that interfaces with the
@@ -1671,7 +1707,7 @@ match_transparent_type (struct symtab *s
       struct symbol *sym;
 
       bv = BLOCKVECTOR (symtab);
-      block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
+      block = BLOCKVECTOR_BLOCK (bv, kind);
       sym = lookup_block_symbol (block, name, STRUCT_DOMAIN);
       if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym)))
 	return sym;
@@ -1694,13 +1730,19 @@ basic_lookup_transparent_type (const cha
   struct blockvector *bv;
   struct objfile *objfile;
   struct block *block;
-  struct type *t;
+  struct type *type;
+  const char *psym_search_name;
+  struct cleanup *cleanup;
 
   /* Now search all the global symbols.  Do the symtab's first, then
      check the psymtab's. If a psymtab indicates the existence
      of the desired name as a global, then do psymtab-to-symtab
      conversion on the fly and return the found symbol.  */
 
+  psym_search_name = psymtab_search_name (name);
+  cleanup = make_cleanup (xfree, (void *) psym_search_name);
+
+  type = NULL;
   ALL_OBJFILES (objfile)
   {
     ALL_OBJFILE_SYMTABS (objfile, s)
@@ -1711,7 +1753,8 @@ basic_lookup_transparent_type (const cha
 	  sym = lookup_block_symbol (block, name, STRUCT_DOMAIN);
 	  if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym)))
 	    {
-	      return SYMBOL_TYPE (sym);
+	      type = SYMBOL_TYPE (sym);
+	      goto done;
 	    }
 	}
 
@@ -1719,20 +1762,24 @@ basic_lookup_transparent_type (const cha
       {
 	sym
 	  = objfile->sf->qf->expand_one_symtab_matching (objfile,
-							 GLOBAL_BLOCK, name,
+							 GLOBAL_BLOCK,
+							 psym_search_name,
 							 STRUCT_DOMAIN,
 							 match_transparent_type,
 							 NULL);
 	if (sym)
-	  return SYMBOL_TYPE (sym);
+	  {
+	    type = SYMBOL_TYPE (sym);
+	    goto done;
+	  }
       }
   }
 
   ALL_OBJFILES (objfile)
   {
-    t = basic_lookup_transparent_type_quick (objfile, GLOBAL_BLOCK, name);
-    if (t)
-      return t;
+    type = basic_lookup_transparent_type_quick (objfile, GLOBAL_BLOCK, name);
+    if (type)
+      goto done;
   }
 
   /* Now search the static file-level symbols.
@@ -1752,7 +1799,8 @@ basic_lookup_transparent_type (const cha
 	sym = lookup_block_symbol (block, name, STRUCT_DOMAIN);
 	if (sym && !TYPE_IS_OPAQUE (SYMBOL_TYPE (sym)))
 	  {
-	    return SYMBOL_TYPE (sym);
+	    type = SYMBOL_TYPE (sym);
+	    goto done;
 	  }
       }
 
@@ -1760,23 +1808,29 @@ basic_lookup_transparent_type (const cha
       {
 	sym
 	  = objfile->sf->qf->expand_one_symtab_matching (objfile,
-							 STATIC_BLOCK, name,
+							 STATIC_BLOCK,
+							 psym_search_name,
 							 STRUCT_DOMAIN,
 							 match_transparent_type,
 							 NULL);
 	if (sym)
-	  return SYMBOL_TYPE (sym);
+	  {
+	    type = SYMBOL_TYPE (sym);
+	    goto done;
+	  }
       }
   }
 
   ALL_OBJFILES (objfile)
   {
-    t = basic_lookup_transparent_type_quick (objfile, STATIC_BLOCK, name);
-    if (t)
-      return t;
+    type = basic_lookup_transparent_type_quick (objfile, STATIC_BLOCK, name);
+    if (type)
+      goto done;
   }
 
-  return (struct type *) 0;
+ done:
+  do_cleanups (cleanup);
+  return type;
 }