RE: [PATCH] Cilk Plus Array Notation for C++

["Iyer, Balaji V" <balaji dot v dot iyer at intel dot com>]

Sorry, I forgot to fix an header comment to cp_parser_array_notation (...). Here is the fixed patch and the Changelogs again..

Thanks,

Balaji V. Iyer.

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Iyer, Balaji V
> Sent: Thursday, June 27, 2013 11:40 AM
> To: Jason Merrill; Richard Henderson
> Cc: Aldy Hernandez; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] Cilk Plus Array Notation for C++
> 
> Hello Jason et al.,
> 	Please see my responses to your questions below. I am attaching a fixed
> patch and ChangeLog. Is this Ok for trunk?
> 
> Thanks,
> 
> Balaji V. Iyer.
> 
> > -----Original Message-----
> > From: Jason Merrill [mailto:jason@redhat.com]
> > Sent: Wednesday, June 26, 2013 4:13 PM
> > To: Iyer, Balaji V; Richard Henderson
> > Cc: Aldy Hernandez; gcc-patches@gcc.gnu.org
> > Subject: Re: [PATCH] Cilk Plus Array Notation for C++
> >
> > On 06/26/2013 01:31 PM, Iyer, Balaji V wrote:
> > > 	Attached, please find a fixed patch and ChangeLog entries:
> >
> > This patch seems to be missing some hunks that are described in the
> > ChangeLog and were present in the previous patch, such as
> >
> > >         * cp-array-notation.c (cp_length_mismatch_in_expr_p): Combined two
> > >         if statements into one and compared integers using tree_int_cst_equal.
> >
> 
> I checked in a patch for C that replaced all dynamic arrays in C with vec_trees.
> Thus, this function could be replaced with the function in c-family. It was my
> mistake that I forgot to take this line out of the changelog entry.
> 
> > > Sorry about that, I accidentally missed this one. It is fixed now. I
> > > have also
> > added the braced list capability into cp_array_notation.
> >
> > Hmm, I seem to have been unclear.  I was expecting that the call to
> > cp_parser_array_notation could come after the braced list case, so we
> > don't need to duplicate the offsetof or braced list code inside
> > cp_parser_array_notation.
> >
> > And then if you'd like we could check for ':' before the ']' and give
> > a helpful diagnostic.
> 
> I Think I have fixed this as you requested.
> 
> >
> > > +      /* If we hare here, then there are 2 possibilities:
> >
> > "are"
> 
> Fixed!
> 
> >
> > > +  if (processing_template_decl)
> > > +    array_ntn_expr = build_min_nt_loc (loc, ARRAY_NOTATION_REF, array,
> > > +                                      start_index, length, stride,
> > > + NULL_TREE);
> >
> > If we know the type of the array, we should use it, rather than always
> > leaving it null in a template.  That is, if !dependent_type_p (type),
> > we should give the ARRAY_NOTATION_REF a real type.
> >
> 
> Fixed.
> 
> > > +      if (TREE_CODE (array_type) == RECORD_TYPE
> > > +         || TREE_CODE (array_type) == POINTER_TYPE)
> > >          {
> > > +         error_at (loc, "start-index and length fields necessary for "
> > > +                   "using array notation in pointers or records");
> >
> 
> Moved as you suggested.
> 
> > In a template, array_type might be NULL_TREE; this diagnostic should
> > move into build_array_notation_ref.
> >
> > > +      array_type_domain = TYPE_DOMAIN (array_type);
> > > +      if (!array_type_domain)
> > > +       {
> > > +         error_at (loc, "start-index and length fields necessary for "
> > > +                   "using array notation with array of unknown bound");
> > > +         cp_parser_skip_to_end_of_statement (parser);
> > > +         return error_mark_node;
> > > +       }
> > > +      start_index = TYPE_MINVAL (array_type_domain);
> > > +      start_index = cp_fold_convert (ptrdiff_type_node, start_index);
> > > +      length_index = size_binop
> > > +       (PLUS_EXPR, TYPE_MAXVAL (array_type_domain), size_one_node);
> > > +      length_index = cp_fold_convert (ptrdiff_type_node, length_index);
> > > +      stride = build_int_cst (ptrdiff_type_node, 1);
> >
> > As should all this code.  cp_parser_array_notation should only parse.
> >
> 
> Moved as you suggested.
> 
> > > +      else
> > > +       stride = build_one_cst (ptrdiff_type_node);
> >
> > I would move this into build_array_notation_ref as well.
> 
> Moved as you suggested.
> 
> >
> > Jason

Attachment: diff.txt
Description: diff.txt

Attachment: CL.txt
Description: CL.txt