Please see my response below: > -----Original Message----- > From: Aldy Hernandez [mailto:al...@redhat.com] > Sent: Thursday, March 21, 2013 10:25 AM > To: Joseph S. Myers > Cc: Iyer, Balaji V; gcc-patches > Subject: Re: [patch] cilkplus array notation for C (clean, independent > patchset, > take 1) > > > > I have found some little nits that I will point out in a reply to this > > message. > > Balaji, in Joseph's last review he mentioned: > > > In find_rank you have error ("Rank Mismatch!"); - this is not a > > properly formatted error message according to the GNU Coding standards > > (which typically would not have any uppercase). I'd also suggest that > > when you find a rank, you store (through a location_t * pointer) the > > location of the first expression found with that rank, so if you then > > find a mismatching rank you can use error_at to point to that rank and > > then inform to point to the previous rank it didn't match. > > I see you have dispensed with the rank mismatch error altogether. Was this on > purpose? For example, you now have: > > > for (ii_tree = array; > > ii_tree && TREE_CODE (ii_tree) == ARRAY_NOTATION_REF; > > ii_tree = ARRAY_NOTATION_ARRAY (ii_tree)) > > current_rank++; > > if (*rank == 0) > > *rank = current_rank; > > Which is basically failing to set *rank when it's already set (with no > error). Is this > on purpose? If so, can you explain? > > If it's on purpose, document it in the comment at the top of the function. > And > then, why don't you exit the function immediately upon entry if *rank is non- > zero? It's a waste of time to do the rest of the analysis if you're just > going to > throw it away. > > Furthermore, Joseph suggested you store the location of the initial rank so > you > can give a meaningful error message later and tell the user where the mismatch > is occurring and where the original rank occurred.
I believe I have done what Joseph mentioned. If you would look at line 473 c-array-notation.c it mentions when LHS is scalar while RHS is not (which is unacceptable). Also, if they both have array notations and there is a rank mismatch, then it is pointed out in line 490 of c-array-notation.c. > > Aldy
