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

Message ID 1397603337-28016-1-git-send-email-cody@linux.vnet.ibm.com
State Mainlined, archived
Headers show

Commit Message

cody@linux.vnet.ibm.com April 15, 2014, 11:08 p.m. UTC
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

Comments

Josh Triplett April 17, 2014, 12:12 a.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
cody@linux.vnet.ibm.com April 17, 2014, 3:50 a.m. UTC | #2
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.

--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Josh Triplett April 17, 2014, 5:09 a.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

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
+ */