| Message ID | CAJXstsCABPFqxqYtKUZquR1PptovvOVVhRHZ9y5yGTEQ4rkwBw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Oct 25, 2014 at 8:16 PM, Prathamesh Kulkarni <bilbotheelffriend@gmail.com> wrote: > Currently this is accepted: > (for plus (mult div) > ...) > which is incorrect. This patch puts more error-checks on the user-defined > operator in for-pattern. Nicely spotted issue. But I prefer to fix it simpler like with the following which I applied. Richard. 2014-10-28 Richard Biener <rguenther@suse.de> * genmatch.c (parser::parse_for): Properly check for already defined operators. > * genmatch.c > (insert_operator): New function. > (parse_for): Call insert_operator. > > Thanks, > Prathamesh
On Tue, Oct 28, 2014 at 3:45 PM, Richard Biener <richard.guenther@gmail.com> wrote: > On Sat, Oct 25, 2014 at 8:16 PM, Prathamesh Kulkarni > <bilbotheelffriend@gmail.com> wrote: >> Currently this is accepted: >> (for plus (mult div) >> ...) >> which is incorrect. This patch puts more error-checks on the user-defined >> operator in for-pattern. > > Nicely spotted issue. But I prefer to fix it simpler like with the following > which I applied. Won't using get_operator() lookup the hash-table twice for a valid user-defined operator ? I thought it would be be better instead to write another function (insert_operator) to insert operator into hash-table, that does one lookup. Thanks, Prathamesh. > > Richard. > > 2014-10-28 Richard Biener <rguenther@suse.de> > > * genmatch.c (parser::parse_for): Properly check for already > defined operators. > > >> * genmatch.c >> (insert_operator): New function. >> (parse_for): Call insert_operator. >> >> Thanks, >> Prathamesh
On Tue, Oct 28, 2014 at 11:27 AM, Prathamesh Kulkarni <bilbotheelffriend@gmail.com> wrote: > On Tue, Oct 28, 2014 at 3:45 PM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Sat, Oct 25, 2014 at 8:16 PM, Prathamesh Kulkarni >> <bilbotheelffriend@gmail.com> wrote: >>> Currently this is accepted: >>> (for plus (mult div) >>> ...) >>> which is incorrect. This patch puts more error-checks on the user-defined >>> operator in for-pattern. >> >> Nicely spotted issue. But I prefer to fix it simpler like with the following >> which I applied. > Won't using get_operator() lookup the hash-table twice for a valid > user-defined operator ? Sure - but who cares for a generator program. > I thought it would be be better instead to write another function > (insert_operator) to insert operator into hash-table, that does one > lookup. Well - I'd prefer easy maintainance for the code over optimization ;) Which means less lines of code == better. Thanks, Richard. > Thanks, > Prathamesh. >> >> Richard. >> >> 2014-10-28 Richard Biener <rguenther@suse.de> >> >> * genmatch.c (parser::parse_for): Properly check for already >> defined operators. >> >> >>> * genmatch.c >>> (insert_operator): New function. >>> (parse_for): Call insert_operator. >>> >>> Thanks, >>> Prathamesh
Index: gcc/genmatch.c =================================================================== --- gcc/genmatch.c (revision 216673) +++ gcc/genmatch.c (working copy) @@ -349,8 +349,41 @@ return 0; } +id_base * +insert_operator (id_base *op) +{ + id_base **orig_slot; + id_base **slot = orig_slot = operators->find_slot_with_hash (op, op->hashval, INSERT); + if (*slot) + return 0; + char *id2 = xstrdup (op->id); + for (unsigned i = 0; i < strlen (id2); ++i) + id2[i] = TOUPPER (id2[i]); + id_base tem (id_base::CODE, id2); + slot = operators->find_slot_with_hash (&tem, tem.hashval, INSERT); + if (*slot) + { + free (id2); + return 0; + } + + /* Try _EXPR appended */ + id2 = (char *)xrealloc (id2, strlen (id2) + sizeof ("_EXPR") + 1); + strcat (id2, "_EXPR"); + id_base tem2 (id_base::CODE, id2); + slot = operators->find_slot_with_hash (&tem2, tem2.hashval, INSERT); + + if (*slot) + { + free (id2); + return 0; + } + + *orig_slot = op; + return *orig_slot; +} /* The AST produced by parsing of the pattern definitions. */ struct dt_operand; @@ -2920,10 +2953,8 @@ /* Insert the user defined operators into the operator hash. */ const char *id = get_ident (); user_id *op = new user_id (id); - id_base **slot = operators->find_slot_with_hash (op, op->hashval, INSERT); - if (*slot) - fatal_at (token, "operator already defined"); - *slot = op; + if (insert_operator (op) == 0) + fatal_at (token, "operator '%s' already defined", id); user_ids.safe_push (op); eat_token (CPP_OPEN_PAREN);