This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Cilk Plus Array Notation for C++
- From: Jason Merrill <jason at redhat dot com>
- To: "Iyer, Balaji V" <balaji dot v dot iyer at intel dot com>, Richard Henderson <rth at redhat dot com>
- Cc: Aldy Hernandez <aldyh at redhat dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Tue, 25 Jun 2013 16:55:35 -0400
- Subject: Re: [PATCH] Cilk Plus Array Notation for C++
- References: <BF230D13CA30DD48930C31D4099330003A42D85F at FMSMSX101 dot amr dot corp dot intel dot com> <51B8A2FA dot 2020404 at redhat dot com> <BF230D13CA30DD48930C31D4099330003A42E4B8 at FMSMSX101 dot amr dot corp dot intel dot com> <51B9EF1D dot 9060505 at redhat dot com> <51B9F0EA dot 50709 at redhat dot com> <BF230D13CA30DD48930C31D4099330003A439163 at FMSMSX101 dot amr dot corp dot intel dot com> <51BF4222 dot 3050107 at redhat dot com> <BF230D13CA30DD48930C31D4099330003A4391EA at FMSMSX101 dot amr dot corp dot intel dot com> <51C0F06E dot 3080507 at redhat dot com> <BF230D13CA30DD48930C31D4099330003A439A73 at FMSMSX101 dot amr dot corp dot intel dot com> <51C341D2 dot 8020707 at redhat dot com> <BF230D13CA30DD48930C31D4099330003A43A479 at FMSMSX101 dot amr dot corp dot intel dot com> <51C5CEC9 dot 8050309 at redhat dot com> <BF230D13CA30DD48930C31D4099330003A43B25B at FMSMSX101 dot amr dot corp dot intel dot com> <51C9AB76 dot 606 at redhat dot com> <BF230D13CA30DD48930C31D4099330003A43B459 at FMSMSX101 dot amr dot corp dot intel dot com>
On 06/25/2013 02:27 PM, Iyer, Balaji V wrote:
This time, I ran the command you gave me. Please tell me how it looks.
No ChangeLog this time, thanks.
Another solution is to replace get_tmp_regvar with get_temporary_var () + add_decl_expr (..). I have implemented this because it looks "more correct"
OK.
I am setting the scope correction to false right before I look for length and restore it right after I parse the scope (i.e. outside the if-statement). I think this should fix the issue.
OK.
I think it would be better to convert start/stride to ptrdiff_t.
I don't think I can do that. Stride can be negative and if I am not mistaken, ptrdiff_t is unsigned.
You are mistaken. :)
ptrdiff_t is the signed version of size_t.
What I had in mind is that in the case of a normal array reference,
cp_parser_array_notation will update index (which is passed by address) and
return NULL_TREE, so that it gets back on the normal path.
It doesn't look like you addressed this comment.
+ if (processing_template_decl)
+ {
+ array_type = TREE_TYPE (array_value);
+ type = TREE_TYPE (array_type);
+ }
We should be able to parse array notation in a template even when the array
expression has unknown type. In a template, just parse and remember the raw
expressions without worrying about diagnostics and conversions.
Or this one.
+ /* If an array's index is an array notation, then its rank cannot be
+ greater than one. */
This one error is much easier to do it here than anywhere else. An array_ref could be a parameter inside a function, part of a modify expression, unary expression etc. If I move it to transformation stage, I have to do checks in all these places and there is a small chance some will slip through the cracks. This is almost a fool proof way of doing it. Such things have been done before. For example, Open MP does a return expression check in finish_return_stmt (even though this is a different issue we are talking about).
What's the failure mode if one is missed? I would expect it to be
pretty obvious.
If it is the code lines that is an issue, then I am willing to enclose that in a function or #define.
But I guess splitting it out into a separate function is OK.
What remaining obstacles are there to sharing most of the expansion code
between C and C++? That can be a separate patch, of course.
Any thoughts?
Jason