On Fri, Apr 25, 2014 at 05:09:57PM +0530, Prathamesh Kulkarni wrote:
> On Thu, Apr 24, 2014 at 9:18 PM, Diego Novillo <dnovillo@google.com> wrote:
> > Please remember to send patches to gcc-patches.
> >
> > I'm wondering if you couldn't just use std::string here.  No need to change
> > one complicated allocation scheme with another.

I was thinking about suggesting that too, a real string APi (and
std::string is the easiest to hand) seems much less error prone.

> Since we were using c-styled strings throughout genmatch, I thought of
> using obstack instead of std::string.
> This patch uses std::string in parse_c_expr to build c-code string.
> Should we also change AST (operand and subclasses) to use std::string
> instead of c-styled strings ?

it might make things nicer, and it would save you the strdup at the end
of this function, not that performance matters here.  So I'd approve of
doing that.

Trev

> 
> Thanks and Regards,
> Prathamesh
> >
> >
> > Diego.
> >
> >
> > On Thu, Apr 24, 2014 at 8:15 AM, Prathamesh Kulkarni
> > <bilbotheelffriend@gmail.com> wrote:
> >>
> >> Hi,
> >>    There was a comment in parse_c_expr, mentioning to use obstack to
> >> build c-code string. I have attached patch for the same.
> >> OK to commit ?
> >>
> >> * genmatch.c (parse_c_expr): Use obstack to build c code string.
> >>
> >> Thanks and Regards,
> >> Prathamesh
> >
> >

> Index: gcc/genmatch.c
> ===================================================================
> --- gcc/genmatch.c	(revision 209470)
> +++ gcc/genmatch.c	(working copy)
> @@ -29,7 +29,7 @@ along with GCC; see the file COPYING3.
>  #include "hashtab.h"
>  #include "hash-table.h"
>  #include "vec.h"
> -
> +#include <string>
>  
>  /* Grammar
>  
> @@ -689,15 +689,17 @@ parse_c_expr (cpp_reader *r, cpp_ttype s
>    cpp_ttype end;
>    unsigned opencnt;
>    char *code;
> +  std::string cstr;
> +
>    eat_token (r, start);
>    if (start == CPP_OPEN_PAREN)
>      {
> -      code = xstrdup ("(");
> +      cstr += "("; 
>        end = CPP_CLOSE_PAREN;
>      }
>    else if (start == CPP_OPEN_BRACE)
>      {
> -      code = xstrdup ("({");
> +      cstr += "({";
>        end = CPP_CLOSE_BRACE;
>      }
>    else
> @@ -714,13 +716,9 @@ parse_c_expr (cpp_reader *r, cpp_ttype s
>  	  if (n->type == CPP_NUMBER
>  	      && !(n->flags & PREV_WHITE))
>  	    {
> -	      code = (char *)xrealloc (code, strlen (code)
> -				       + strlen ("captures[") + 4);
>  	      if (token->flags & PREV_WHITE)
> -		strcat (code, " ");
> -	      strcat (code, "captures[");
> -	      strcat (code, get_number (r));
> -	      strcat (code, "]");
> +		cstr += " ";	
> +	      cstr.append ("captures[").append (get_number (r)).append ("]");
>  	      continue;
>  	    }
>  	}
> @@ -734,24 +732,19 @@ parse_c_expr (cpp_reader *r, cpp_ttype s
>  
>        /* Output the token as string.  */
>        char *tk = (char *)cpp_token_as_text (r, token);
> -      code = (char *)xrealloc (code, strlen (code) + strlen (tk) + 2);
>        if (token->flags & PREV_WHITE)
> -	strcat (code, " ");
> -      strcat (code, tk);
> +	cstr += " ";	
> +      cstr += tk;
>      }
>    while (1);
>    if (end == CPP_CLOSE_PAREN)
> -    {
> -      code = (char *)xrealloc (code, strlen (code) + 1 + 1);
> -      strcat (code, ")");
> -    }
> +    cstr += ")";
>    else if (end == CPP_CLOSE_BRACE)
> -    {
> -      code = (char *)xrealloc (code, strlen (code) + 1 + 2);
> -      strcat (code, "})");
> -    }
> +    cstr += "})"; 
>    else
>      gcc_unreachable ();
> +
> +  code = (char *) xstrdup (cstr.c_str ());
>    return new c_expr (code);
>  }
>