On Fri, 7 Aug 2020, Martin Sebor via Gcc-patches wrote:
> > I don't see anything in the tests in this patch to cover this sort of case
> > (arrays of pointers, including arrays of pointers to arrays etc.).
>
> I've added a few test cases and reworked the declarator parsing
> (get_parm_array_spec) a bit, fixing some bugs.
I don't think get_parm_array_spec is yet logically right (and I don't see
tests of the sort of cases I'm concerned about, such as arrays of pointers
to arrays, or pointers with attributes applied to them).
You have logic
+ if (pd->kind == cdk_pointer
+ && (!next || next->kind == cdk_id))
+ {
+ /* Do nothing for the common case of a pointer. The fact that
+ the parameter is one can be deduced from the absence of
+ an arg spec for it. */
+ return attrs;
+ }
which is correct as far as it goes (when it returns with nothing done,
it's correct to do so, because the argument is indeed a pointer), but
incomplete:
* Maybe cdk_pointer is followed by cdk_attrs before cdk_id. In this case
the code won't return.
* Maybe the code is correct to continue because we're in the case of an
array of pointers (cdk_array follows). But as I understand it, the intent
is to set up an "arg spec" that describes only the (multidimensional)
array that is the parameter itself - not any array pointed to. And it
looks to me like, in the case of an array of pointers to arrays, both sets
of array bounds would end up in the spec constructed.
What I think is correct is for both cdk_pointer and cdk_function to result
in the spec built up so far being cleared (regardless of what follows
cdk_pointer or cdk_function), rather than early return, so that the spec
present at the end is for the innermost sequence of array declarators
(possibly with attributes involved as well). (cdk_function shouldn't
actually be an issue, since functions can't return arrays or functions,
but logically it seems appropriate to treat it like cdk_pointer.)
Then, the code
+ if (pd->kind == cdk_id)
+ {
+ /* Extract the upper bound from a parameter of an array type. */
also seems misplaced. If the type specifiers for the parameter are a
typedef for an array type, that array type should be processed *before*
the declarator to get the correct semantics (as if the bounds from those
type specifiers were given in the declarator), not at the end which gets
that type out of order with respect to array declarators. (Processing
before the declarator also means clearing the results of that processing
if a pointer declarator is encountered at any point, because in that case
the array type in the type specifiers is irrelevant.)
The logic
+ /* Skip all constant bounds except the most significant one.
+ The interior ones are included in the array type. */
+ if (next && (next->kind == cdk_array || next->kind == cdk_pointer))
+ continue;
is another example of code that fails to look past cdk_attrs.
--
Joseph S. Myers
jos...@codesourcery.com
