[PATCH] parse: support c99 [static ...] in abstract array declarators

[PATCH] parse: support c99 [static ...] in abstract array declarators

[Cody P Schafer]

Makes sparse a little more accepting than the standard: we accept any
number of ["static", "restrict"] repeated in any order, while the n1570
specifies (in 6.7.6.2.3) that either type-qualifiers (ie: "restrict")
come first and are followed by "static" or the opposite ("static" then
type-qualifiers).

Also add a test.

Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
---
 ident-list.h                                  | 1 +
 parse.c                                       | 2 +-
 validation/abstract-array-declarator-static.c | 6 ++++++
 3 files changed, 8 insertions(+), 1 deletion(-)
 create mode 100644 validation/abstract-array-declarator-static.c

diff --git a/ident-list.h b/ident-list.h
index e93aae7..c0fc18f 100644
--- a/ident-list.h
+++ b/ident-list.h
@@ -91,6 +91,7 @@ IDENT(artificial); IDENT(__artificial__);
 IDENT(leaf); IDENT(__leaf__);
 IDENT(vector_size); IDENT(__vector_size__);
 IDENT(error); IDENT(__error__);
+IDENT(static);
 
 
 /* Preprocessor idents.  Direct use of __IDENT avoids mentioning the keyword
diff --git a/parse.c b/parse.c
index 9cc5f65..cbe3af4 100644
--- a/parse.c
+++ b/parse.c
@@ -1532,7 +1532,7 @@ static struct token *abstract_array_declarator(struct token *token, struct symbo
 {
 	struct expression *expr = NULL;
 
-	if (match_idents(token, &restrict_ident, &__restrict_ident, NULL))
+	while (match_idents(token, &restrict_ident, &__restrict_ident, &static_ident, NULL))
 		token = token->next;
 	token = parse_expression(token, &expr);
 	sym->array_size = expr;
diff --git a/validation/abstract-array-declarator-static.c b/validation/abstract-array-declarator-static.c
new file mode 100644
index 0000000..23cbae0
--- /dev/null
+++ b/validation/abstract-array-declarator-static.c
@@ -0,0 +1,6 @@
+
+extern void f(int g[static 1]);
+
+/*
+ * check-name: abstract array declarator static
+ */
-- 
1.9.2

Re: [PATCH] parse: support c99 [static ...] in abstract array declarators

[Josh Triplett]

On Tue, Apr 15, 2014 at 04:08:57PM -0700, Cody P Schafer wrote:
> Makes sparse a little more accepting than the standard: we accept any
> number of ["static", "restrict"] repeated in any order, while the n1570
> specifies (in 6.7.6.2.3) that either type-qualifiers (ie: "restrict")
> come first and are followed by "static" or the opposite ("static" then
> type-qualifiers).
> 
> Also add a test.
> 
> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>

What's the rationale for this?  Why should sparse accept more than the
standard allows?  What real-world code do you have that requires this?

And would it be worth adding a warning for this non-standards-compliant
code, even if that warning isn't on by default?

- Josh Triplett

Re: [PATCH] parse: support c99 [static ...] in abstract array declarators

[Cody P Schafer]

On 04/16/2014 05:12 PM, Josh Triplett wrote:
> On Tue, Apr 15, 2014 at 04:08:57PM -0700, Cody P Schafer wrote:
>> Makes sparse a little more accepting than the standard: we accept any
>> number of ["static", "restrict"] repeated in any order, while the n1570
>> specifies (in 6.7.6.2.3) that either type-qualifiers (ie: "restrict")
>> come first and are followed by "static" or the opposite ("static" then
>> type-qualifiers).
>>
>> Also add a test.
>>
>> Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
>
> What's the rationale for this?  Why should sparse accept more than the
> standard allows?  What real-world code do you have that requires this?
>
> And would it be worth adding a warning for this non-standards-compliant
> code, even if that warning isn't on by default?

I could have sparse be just as strict as the standard, it just was just 
(much) simpler to make it liberal in what it accepts. If you're fine 
with some more verbose code, I'll put together something that is stricter.

Re: [PATCH] parse: support c99 [static ...] in abstract array declarators

[Josh Triplett]

On Wed, Apr 16, 2014 at 08:50:23PM -0700, Cody P Schafer wrote:
> On 04/16/2014 05:12 PM, Josh Triplett wrote:
> >On Tue, Apr 15, 2014 at 04:08:57PM -0700, Cody P Schafer wrote:
> >>Makes sparse a little more accepting than the standard: we accept any
> >>number of ["static", "restrict"] repeated in any order, while the n1570
> >>specifies (in 6.7.6.2.3) that either type-qualifiers (ie: "restrict")
> >>come first and are followed by "static" or the opposite ("static" then
> >>type-qualifiers).
> >>
> >>Also add a test.
> >>
> >>Signed-off-by: Cody P Schafer <cody@linux.vnet.ibm.com>
> >
> >What's the rationale for this?  Why should sparse accept more than the
> >standard allows?  What real-world code do you have that requires this?
> >
> >And would it be worth adding a warning for this non-standards-compliant
> >code, even if that warning isn't on by default?
> 
> I could have sparse be just as strict as the standard, it just was just
> (much) simpler to make it liberal in what it accepts. If you're fine with
> some more verbose code, I'll put together something that is stricter.

I'd suggest trying to match the standard in this case, or failing that
match what GCC does.

- Josh Triplett

Re: [PATCH] parse: support c99 [static ...] in abstract array declarators

[Christopher Li]

On Wed, Apr 16, 2014 at 10:09 PM, Josh Triplett <josh@joshtriplett.org> wrote:
>> I could have sparse be just as strict as the standard, it just was just
>> (much) simpler to make it liberal in what it accepts. If you're fine with
>> some more verbose code, I'll put together something that is stricter.
>
> I'd suggest trying to match the standard in this case, or failing that
> match what GCC does.

I second that. It does not seem too complicate to make sparse accept
"static" in two possible position.

Here is a purpose patch with limited test. I expand the test to cover
more static test case.

Cody, does this patch work for you?

Chris

diff --git a/parse.c b/parse.c
index 70553f2..76aba40 100644
--- a/parse.c
+++ b/parse.c
@@ -1533,12 +1533,28 @@ static struct token
*declaration_specifiers(struct token *tok
        return token;
 }

+static struct token *abstract_array_static_declarator(struct token
*token, int *has_
+{
+       while (token->ident == &static_ident) {
+               if (*has_static)
+                       warning(token->pos, "duplicate array static
declarator");
+
+               *has_static = 1;
+               token = token->next;
+       }
+       return token;
+
+}
+
 static struct token *abstract_array_declarator(struct token *token,
struct symbol *s
 {
        struct expression *expr = NULL;
+       int has_static = 0;

-       while (match_idents(token, &restrict_ident, &__restrict_ident,
&static_ident,
-               token = token->next;
+       token = abstract_array_static_declarator(token, &has_static);
+
+       if (match_idents(token, &restrict_ident, &__restrict_ident, NULL))
+               token = abstract_array_static_declarator(token->next,
&has_static);
        token = parse_expression(token, &expr);
        sym->array_size = expr;
        return token;
diff --git a/validation/abstract-array-declarator-static.c
b/validation/abstract-arra
index 23cbae0..b0af17b 100644
--- a/validation/abstract-array-declarator-static.c
+++ b/validation/abstract-array-declarator-static.c
@@ -1,6 +1,14 @@

-extern void f(int g[static 1]);
+extern void f1(int g[static 1]);
+extern void f2(int g[static restrict 1]);
+extern void f3(int g[restrict static 1]);
+extern void f4(int g[static restrict static 1]);       /* duplicate
static error */
+extern void f5(int g[restrict static static 1]);       /* duplicate
static error */

 /*
  * check-name: abstract array declarator static
+ * check-error-start
+abstract-array-declarator-static.c:5:38: warning: duplicate array
static declarator
+abstract-array-declarator-static.c:6:38: warning: duplicate array
static declarator
+ * check-error-end
  */

Re: [PATCH] parse: support c99 [static ...] in abstract array declarators

[Christopher Li]

On Wed, Apr 16, 2014 at 11:50 PM, Christopher Li <sparse@chrisli.org> wrote:

> Here is a purpose patch with limited test. I expand the test to cover
> more static test case.
>
> Cody, does this patch work for you?

I apply the patch and push it out.

Please let me know if it breaks any thing.

Chris

Re: [PATCH] parse: support c99 [static ...] in abstract array declarators

[Cody P Schafer]

On 04/17/2014 10:52 PM, Christopher Li wrote:
> On Wed, Apr 16, 2014 at 11:50 PM, Christopher Li <sparse@chrisli.org> wrote:
>
>> Here is a purpose patch with limited test. I expand the test to cover
>> more static test case.
>>
>> Cody, does this patch work for you?
>
> I apply the patch and push it out.
>
> Please let me know if it breaks any thing.
>

Looks fine to me Chris, thanks for putting this together.