On Fri, Apr 25, 2014 at 2:01 PM, Trevor Saunders <tsaund...@mozilla.com
wrote:
> On Fri, Apr 25, 2014 at 05:09:57PM +0530, Prathamesh Kulkarni wrote:
>> On Thu, Apr 24, 2014 at 9:18 PM, Diego Novillo <dnovi...@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.
Indeed. It's only a generator program, so using std::string is fine.
Thanks,
Richard.
> Trev
>
>>
>> Thanks and Regards,
>> Prathamesh
>> >
>> >
>> > Diego.
>> >
>> >
>> > On Thu, Apr 24, 2014 at 8:15 AM, Prathamesh Kulkarni
>> > <bilbotheelffri...@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);
>> }
>>
>
