Thanks for the review! New patch attached.

On Sat, Aug 11, 2012 at 8:14 PM, John McCall <[email protected]> wrote:
> +        unsigned x = chunkIndex;
> +        for (;;) {
> +          // Walk outwards along the declarator chunks.
> +          if (x == 0)
> +            break;
> +          x--;
>
> This can just be
>   while (x != 0) {
>     x--;

Yeah, that's better.

> +          switch (DC.Kind) {
> +          case DeclaratorChunk::Paren:
> +            continue;
> +          case DeclaratorChunk::Array:
> +          case DeclaratorChunk::Pointer:
>
> We try to avoid 'default' cases, and this is a good example why.  I claim
> that there are no declarator chunks which are actually valid here except
> Paren.

Right.

> Suppressing the error when the chunk is a Function or BlockPointer
> is fine, since we'll still get the more-important diagnostic that the type is
> invalid.  We should not suppress the error when the chunk is Reference
> or MemberPointer, though.

I was thinking that in these cases we would get errors about trying to
use a C99 feature in C++, but maybe it's better not to ignore it.

> Also, if someone adds a new declarator chunk
> (not entirely impossible), they should have to consider what to do in this
> case.

That's a good point.

Thanks,
Hans

Attachment: static_in_array_decls3.patch
Description: Binary data

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to