[v5] sparse: add support for _Static_assert

Commit Message

Christopher Li May 6, 2017, 8:22 p.m. UTC

I play with it a bit.

On Thu, May 4, 2017 at 12:00 PM, Lance Richardson <[e-mail]> wrote:
> +static int match_static_assert(struct token *token)
> +{
> +       return token_type(token) == TOKEN_IDENT &&
> +              token->ident == &_Static_assert_ident;
> +}

I just find out there is already function match_ident() so use that
instead will make it clear it is compare against "_Static_assert".
Sorry I forget about this function earlier. I only find it when I play
with the code today.

>
>  struct statement *alloc_statement(struct position pos, int type)
>  {
> @@ -1864,13 +1877,17 @@ static struct token *declaration_list(struct token *token, struct symbol_list **
>  static struct token *struct_declaration_list(struct token *token, struct symbol_list **list)
>  {
>         while (!match_op(token, '}')) {
> -               if (!match_op(token, ';'))
> -                       token = declaration_list(token, list);
> -               if (!match_op(token, ';')) {
> -                       sparse_error(token->pos, "expected ; at end of declaration");
> -                       break;
> +               if (match_static_assert(token))
> +                       token = parse_static_assert(token, NULL);
> +               else {
> +                       if (!match_op(token, ';'))
> +                               token = declaration_list(token, list);
> +                       if (!match_op(token, ';')) {
> +                               sparse_error(token->pos, "expected ; at end of declaration");
> +                               break;
> +                       }
> +                       token = token->next;

I reshape the code a bit to make it looks compact. Please see my
attachment patch.

> +static struct token *parse_static_assert(struct token *token, struct symbol_list **unused)
> +{
> +       struct expression *expr1 = NULL, *expr2 = NULL;
> +       int val;
> +
> +       token = expect(token->next, '(', "after _Static_assert");
> +       token = constant_expression(token, &expr1);
> +       token = expect(token, ',', "after first argument of _Static_assert");
> +       token = parse_expression(token, &expr2);
> +       token = expect(token, ')', "after second argument of _Static_assert");
> +
> +        token = expect(token, ';', "after _Static_assert()");
> +
> +       if (!expr1 || !expr2)
> +               return token;

OK,  this parsing does not complain about the following input:
_Static_assert(0, );
_Static_assert(, "");

I fix it in my patch. While I am there, I rename the expr1 to "cond".


>  /* Make a statement out of an expression */
>  static struct statement *make_statement(struct expression *expr)
>  {
> @@ -2389,11 +2436,14 @@ static struct token * statement_list(struct token *token, struct statement_list
>                         }
>                         stmt = alloc_statement(token->pos, STMT_DECLARATION);
>                         token = external_declaration(token, &stmt->declaration);
> +                       add_statement(list, stmt);
> +               } else if (match_static_assert(token)) {
> +                       token = parse_static_assert(token, NULL);

After take a closer look of the grimmer syntax. Static assert is
actually a declaration rather than a statement. So I merge this
two because external_declaration() can handle _Static_assert
parsing any way. Just need to pay attention it does not generate
resulting symbol node.

I rename the patch as V6. V5 does not apply to master any more.

Luc, can you take a look the V6 patch as well?

Thanks

Chris

Comments

Luc Van Oostenryck May 6, 2017, 11:11 p.m. UTC | #1

On Sat, May 06, 2017 at 04:22:04PM -0400, Christopher Li wrote:
> I play with it a bit.
> 
> On Thu, May 4, 2017 at 12:00 PM, Lance Richardson <[e-mail]> wrote:
> > +static int match_static_assert(struct token *token)
> > +{
> > +       return token_type(token) == TOKEN_IDENT &&
> > +              token->ident == &_Static_assert_ident;
> > +}
> 
> I just find out there is already function match_ident() so use that
> instead will make it clear it is compare against "_Static_assert".
> Sorry I forget about this function earlier. I only find it when I play
> with the code today.

Yes, it's even better with match_ident().

> >
> >  struct statement *alloc_statement(struct position pos, int type)
> >  {
> > @@ -1864,13 +1877,17 @@ static struct token *declaration_list(struct token *token, struct symbol_list **
> >  static struct token *struct_declaration_list(struct token *token, struct symbol_list **list)
> >  {
> >         while (!match_op(token, '}')) {
> > -               if (!match_op(token, ';'))
> > -                       token = declaration_list(token, list);
> > -               if (!match_op(token, ';')) {
> > -                       sparse_error(token->pos, "expected ; at end of declaration");
> > -                       break;
> > +               if (match_static_assert(token))
> > +                       token = parse_static_assert(token, NULL);
> > +               else {
> > +                       if (!match_op(token, ';'))
> > +                               token = declaration_list(token, list);
> > +                       if (!match_op(token, ';')) {
> > +                               sparse_error(token->pos, "expected ; at end of declaration");
> > +                               break;
> > +                       }
> > +                       token = token->next;
> 
> I reshape the code a bit to make it looks compact. Please see my
> attachment patch.

Yes, it's the same as I did.

> > +static struct token *parse_static_assert(struct token *token, struct symbol_list **unused)
> > +{
> > +       struct expression *expr1 = NULL, *expr2 = NULL;
> > +       int val;
> > +
> > +       token = expect(token->next, '(', "after _Static_assert");
> > +       token = constant_expression(token, &expr1);
> > +       token = expect(token, ',', "after first argument of _Static_assert");
> > +       token = parse_expression(token, &expr2);
> > +       token = expect(token, ')', "after second argument of _Static_assert");
> > +
> > +        token = expect(token, ';', "after _Static_assert()");
> > +
> > +       if (!expr1 || !expr2)
> > +               return token;
> 
> OK,  this parsing does not complain about the following input:
> _Static_assert(0, );
> _Static_assert(, "");

Ohoh, indeed.
 
> I fix it in my patch. While I am there, I rename the expr1 to "cond".

Good.
But I have two comments on your patch (I included it here below).

> @@ -2093,6 +2107,40 @@ static struct token *parse_asm_declarator(struct token *token, struct decl_state
> +
> +static struct token *parse_static_assert(struct token *token, struct symbol_list **unused)
> +{
> +	struct expression *cond = NULL, *fail_string = NULL;
> +	int val;
> +
> +	token = expect(token->next, '(', "after _Static_assert");
> +	token = constant_expression(token, &cond);
> +	if (!cond)
> +		sparse_error(token->pos, "expect expression before '%s' token", show_token(token));

I would prefer to put this validation after the parsing
rather than in the middle but it's a detail.

> +	token = expect(token, ',', "after first argument of _Static_assert");
> +	token = parse_expression(token, &fail_string);
> +	if (!fail_string)
> +		sparse_error(token->pos, "expect expression before '%s' token", show_token(token));

ditto.

> +	token = expect(token, ')', "after second argument of _Static_assert");
> +
> +	expect(token, ';', "after _Static_assert()");
> +
> +	if (!cond || !fail_string)
> +		return token;

This test no become unneeded.
Well, I mean that it would be better to move the tests above here
in place of this one. 

> +
> +	val = const_expression_value(cond);
> +
> +	if (fail_string->type != EXPR_STRING) {
> +		sparse_error(fail_string->pos, "bad string literal");
> +		return token;
> +	}

Same, better to put all these validation together.

> +	if (cond->type == EXPR_VALUE && !val)
> +		sparse_error(cond->pos, "static assertion failed: %s",
> +				show_string(fail_string->string));
> +	return token;
> +}
> +

> 
> >  /* Make a statement out of an expression */
> >  static struct statement *make_statement(struct expression *expr)
> >  {
> > @@ -2389,11 +2436,14 @@ static struct token * statement_list(struct token *token, struct statement_list
> >                         }
> >                         stmt = alloc_statement(token->pos, STMT_DECLARATION);
> >                         token = external_declaration(token, &stmt->declaration);
> > +                       add_statement(list, stmt);
> > +               } else if (match_static_assert(token)) {
> > +                       token = parse_static_assert(token, NULL);
> 
> After take a closer look of the grimmer syntax. Static assert is
> actually a declaration rather than a statement. So I merge this
> two because external_declaration() can handle _Static_assert
> parsing any way.

Well, yes but it's a very strange 'declaration'. I had also
looked if both could be merged and concluded that they should
not for two reasons:
1) the grammar of a declaration and the static-assert has one
   difference in the way the final ';' is handled.
   It's included in the spec of static-assert while it's left
   out for declaration (but included in declaration-list)
   With your change here, sparse will complain twice when a
   trailing ';' is missing, and if you remove the "expect(';')"
   in parse_static_assert then:
    - you will have a warning 'Expected ; at the end of type declaration'
      for something that nobody will consider to be a declaration
      (well ok, nobody but the ones that read such details in the
      C grammar).
    - is there situations where a 'declaration' need not to be
      terminated by a ';' ?
2) I think that _Static_assert() should not be considered as a
   declaration but as a statement regarding -Wdeclaration-after-statement.
   Lance's version did this but yours doesn't anymore.
   Of course in the standard they can mix the two in the spec
   since it's OK for the standard to mix decls and statements.
   But that's not true anymore with -Wdeclaration-after-statement
   I certainly want to use the following code without a warning
   'mixing declarations and code' but I want to have this warning
   when a 'true' declaration is mixed with statements.

Maybe these two points can be handled by adding a few conditions
but will the result be clearer than Lance's version?

Also, that's just tow points I saw when seeing you have merged the
two, maybe there is others points that need consideration.

> Just need to pay attention it does not generate
> resulting symbol node.
> 
> I rename the patch as V6. V5 does not apply to master any more.
> 
> Luc, can you take a look the V6 patch as well?
>
> Thanks

Done :)

-- Luc
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Christopher Li May 7, 2017, 12:24 a.m. UTC | #2

On Sat, May 6, 2017 at 7:11 PM, Luc Van Oostenryck
<[e-mail]> wrote:
>
>> @@ -2093,6 +2107,40 @@ static struct token *parse_asm_declarator(struct token *token, struct decl_state
>> +
>> +static struct token *parse_static_assert(struct token *token, struct symbol_list **unused)
>> +{
>> +     struct expression *cond = NULL, *fail_string = NULL;
>> +     int val;
>> +
>> +     token = expect(token->next, '(', "after _Static_assert");
>> +     token = constant_expression(token, &cond);
>> +     if (!cond)
>> +             sparse_error(token->pos, "expect expression before '%s' token", show_token(token));
>
> I would prefer to put this validation after the parsing
> rather than in the middle but it's a detail.

It can be done but there will be more bookkeeping of the token->pos.
We need to remember which token is the one the error happening.

That is why I do earlier. I could make cond_token vs string_token
to save the place. It is a bit annoying when the expression is not there
the token will point the next thing.


>> +     token = expect(token, ')', "after second argument of _Static_assert");
>> +
>> +     expect(token, ';', "after _Static_assert()");
>> +
>> +     if (!cond || !fail_string)
>> +             return token;
>
> This test no become unneeded.
> Well, I mean that it would be better to move the tests above here
> in place of this one.

The test is still required. The previous code only complain but did not
change the execution flow. If the code move here the test can be merged.
Let me try the token_arg thing then.

>
>> +
>> +     val = const_expression_value(cond);
>> +
>> +     if (fail_string->type != EXPR_STRING) {
>> +             sparse_error(fail_string->pos, "bad string literal");
>> +             return token;
>> +     }
>
> Same, better to put all these validation together.

Let me try it then.

> Well, yes but it's a very strange 'declaration'. I had also
> looked if both could be merged and concluded that they should
> not for two reasons:
> 1) the grammar of a declaration and the static-assert has one
>    difference in the way the final ';' is handled.
>    It's included in the spec of static-assert while it's left
>    out for declaration (but included in declaration-list)


>    With your change here, sparse will complain twice when a
>    trailing ';' is missing, and if you remove the "expect(';')"
>    in parse_static_assert then:

Good point about complain twice on the missing ';'.
I am leaning towards remove the expect(';') then.

>     - you will have a warning 'Expected ; at the end of type declaration'
>       for something that nobody will consider to be a declaration
>       (well ok, nobody but the ones that read such details in the
>       C grammar).
If it is only the "type" declaration you are complain about.
We should be able to make it emit "static assert" declaration.
I will see if I can make it better.

>     - is there situations where a 'declaration' need not to be
>       terminated by a ';' ?

It depends, declartion_list() is only use in two places.
Structure and K&R arguments. Both case are terminating with ';'.

Normal external_declare(), I can't think of a case yet.
The stander specify the declaration need to end with ';'.

> 2) I think that _Static_assert() should not be considered as a
>    declaration but as a statement regarding -Wdeclaration-after-statement.
>    Lance's version did this but yours doesn't anymore.

I think the V5 consider static assert not statement nor declaration.

>    Of course in the standard they can mix the two in the spec
>    since it's OK for the standard to mix decls and statements.
>    But that's not true anymore with -Wdeclaration-after-statement
>    I certainly want to use the following code without a warning
>    'mixing declarations and code' but I want to have this warning
>    when a 'true' declaration is mixed with statements.

I think static assert *should not* consider as statements.
It is wrong in may levels.

It is clear that the stander consider static assert as declaration:
    declaration:
        <declaration-specifiers> <init-declarator-list>[opt] ;
        <static_assert-declaration>

I think it is bad mixing the static assert inside statements.
<statement>
_Static_assert();
<statement>

It give the impression the static assert look like a function call.
I see nothing wrong complain about this because stander clearly
state static assert as declaration, for a reason.

Making static assert not a statement nor declaration will further
complicate things.

> Also, that's just tow points I saw when seeing you have merged the
> two, maybe there is others points that need consideration.

That is why we have the review process. I will see if I can come
up with a version without the double ';' error complain.

Thanks for the review. I will see if I can come up with V7 address
your feed back.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Luc Van Oostenryck May 7, 2017, 3:51 a.m. UTC | #3

On Sun, May 7, 2017 at 2:24 AM, Christopher Li <[e-mail]> wrote:
> On Sat, May 6, 2017 at 7:11 PM, Luc Van Oostenryck
> <[e-mail]> wrote:
>>
>>> @@ -2093,6 +2107,40 @@ static struct token *parse_asm_declarator(struct token *token, struct decl_state
>>> +
>>> +static struct token *parse_static_assert(struct token *token, struct symbol_list **unused)
>>> +{
>>> +     struct expression *cond = NULL, *fail_string = NULL;
>>> +     int val;
>>> +
>>> +     token = expect(token->next, '(', "after _Static_assert");
>>> +     token = constant_expression(token, &cond);
>>> +     if (!cond)
>>> +             sparse_error(token->pos, "expect expression before '%s' token", show_token(token));
>>
>> I would prefer to put this validation after the parsing
>> rather than in the middle but it's a detail.
>
> It can be done but there will be more bookkeeping of the token->pos.
> We need to remember which token is the one the error happening.

Ah yes, indeed.
Better to left it there then.

>>     - you will have a warning 'Expected ; at the end of type declaration'
>>       for something that nobody will consider to be a declaration
>>       (well ok, nobody but the ones that read such details in the
>>       C grammar).
> If it is only the "type" declaration you are complain about.
> We should be able to make it emit "static assert" declaration.
> I will see if I can make it better.

No, it's not with "type" that I have a problem but with "declaration".
As I tried to say above: "how many developers will consider the
static assertions as a declaration?"

> I think static assert *should not* consider as statements.
> It is wrong in may levels.
>
> It is clear that the stander consider static assert as declaration:
>     declaration:
>         <declaration-specifiers> <init-declarator-list>[opt] ;
>         <static_assert-declaration>

I don't think this is relevant here.
Yes, in the grammar of the C11 standard static assert is a kind of
declaration. And then?

> I think it is bad mixing the static assert inside statements.
> <statement>
> _Static_assert();
> <statement>
>
> It give the impression the static assert look like a function call.
How is that relevant?
Will this make it look less like a function call:
    int a;
    static_assert(..., ...);
    some_function_call();

> I see nothing wrong complain about this because stander clearly
> state static assert as declaration, for a reason.

The standard has its reasons that don't always match ours.
It's sparse's "raison d'être" to make different choices than the standard.

And for the standard, the whole point is moot anyway since
it allows to freely mix declarations and statements.

Thus the only points that matter are:
- will people want to use static assertions in the middle of statements
  and still would receive a warning when true declarations are mixed
  with statements?
- does code already exists where static assertions are in the middle
  of statements while true declarations are not welcomed there?
- how hard i sit really to add an option to allow one or the other behaviour?

-- Luc
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Lance Richardson May 7, 2017, 3:57 p.m. UTC | #4

> From: "Luc Van Oostenryck" <[e-mail]>
> To: "Christopher Li" <[e-mail]>
> Cc: "Lance Richardson" <[e-mail]>, "Linux-Sparse" <[e-mail]>
> Sent: Saturday, 6 May, 2017 11:51:26 PM
> Subject: Re: [PATCH v5] sparse: add support for _Static_assert
> 
> On Sun, May 7, 2017 at 2:24 AM, Christopher Li <[e-mail]> wrote:
> > On Sat, May 6, 2017 at 7:11 PM, Luc Van Oostenryck
> > <[e-mail]> wrote:
> >>
> >>> @@ -2093,6 +2107,40 @@ static struct token *parse_asm_declarator(struct
> >>> token *token, struct decl_state
> >>> +
> >>> +static struct token *parse_static_assert(struct token *token, struct
> >>> symbol_list **unused)
> >>> +{
> >>> +     struct expression *cond = NULL, *fail_string = NULL;
> >>> +     int val;
> >>> +
> >>> +     token = expect(token->next, '(', "after _Static_assert");
> >>> +     token = constant_expression(token, &cond);
> >>> +     if (!cond)
> >>> +             sparse_error(token->pos, "expect expression before '%s'
> >>> token", show_token(token));
> >>
> >> I would prefer to put this validation after the parsing
> >> rather than in the middle but it's a detail.
> >
> > It can be done but there will be more bookkeeping of the token->pos.
> > We need to remember which token is the one the error happening.
> 
> Ah yes, indeed.
> Better to left it there then.
> 
> >>     - you will have a warning 'Expected ; at the end of type declaration'
> >>       for something that nobody will consider to be a declaration
> >>       (well ok, nobody but the ones that read such details in the
> >>       C grammar).
> > If it is only the "type" declaration you are complain about.
> > We should be able to make it emit "static assert" declaration.
> > I will see if I can make it better.
> 
> No, it's not with "type" that I have a problem but with "declaration".
> As I tried to say above: "how many developers will consider the
> static assertions as a declaration?"
> 
> > I think static assert *should not* consider as statements.
> > It is wrong in may levels.
> >
> > It is clear that the stander consider static assert as declaration:
> >     declaration:
> >         <declaration-specifiers> <init-declarator-list>[opt] ;
> >         <static_assert-declaration>
> 
> I don't think this is relevant here.
> Yes, in the grammar of the C11 standard static assert is a kind of
> declaration. And then?
> 
> > I think it is bad mixing the static assert inside statements.
> > <statement>
> > _Static_assert();
> > <statement>
> >
> > It give the impression the static assert look like a function call.
> How is that relevant?
> Will this make it look less like a function call:
>     int a;
>     static_assert(..., ...);
>     some_function_call();
> 
> > I see nothing wrong complain about this because stander clearly
> > state static assert as declaration, for a reason.
> 
> The standard has its reasons that don't always match ours.
> It's sparse's "raison d'être" to make different choices than the standard.
> 
> And for the standard, the whole point is moot anyway since
> it allows to freely mix declarations and statements.
> 
> Thus the only points that matter are:
> - will people want to use static assertions in the middle of statements
>   and still would receive a warning when true declarations are mixed
>   with statements?
> - does code already exists where static assertions are in the middle
>   of statements while true declarations are not welcomed there?
> - how hard i sit really to add an option to allow one or the other behaviour?
> 
> -- Luc
> 

Hi Chris and Luc,

Thank you very much for your feedback and your patience with this series.

I think if the standard allows mixing static assertions with code, sparse should
also allow it (as both gcc and clang do).

At least gcc does abide by the "static assertions are declarations" idea, e.g.:

    $ gcc -c -Wdeclaration-after-statement moo.c
    moo.c: In function ‘foo’:
    moo.c:11:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
      _Static_assert(1, "");
      ^~~~~~~~~~~~~~

It doesn't appear to be possible to do a similar experiment with clang, in that
case the "ISO C90 forbids mixed declarations and code" warning is only emitted
with "-pedantic C90" or "-pedantic C89", and another warning is produced if
_Static_assert() is used with those options:

   warning: _Static_assert is a C11-specific feature [-Wc11-extensions]

I would be happy to respin, taking the responses to the v5 patch as feedback.

Regards,

   Lance

--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Christopher Li May 7, 2017, 5:34 p.m. UTC | #5

On Sun, May 7, 2017 at 11:57 AM, Lance Richardson <[e-mail]> wrote:
>
> Hi Chris and Luc,
>
> Thank you very much for your feedback and your patience with this series.
>
> I think if the standard allows mixing static assertions with code, sparse should
> also allow it (as both gcc and clang do).
>
> At least gcc does abide by the "static assertions are declarations" idea, e.g.:
>
>     $ gcc -c -Wdeclaration-after-statement moo.c
>     moo.c: In function ‘foo’:
>     moo.c:11:2: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
>       _Static_assert(1, "");
>       ^~~~~~~~~~~~~~

OK. Luc and you have a good point there. So that warning is too strong.

In your V5 patch.

@@ -2436,11 +2483,14 @@ static struct token * statement_list(struct
token *token, struct statement_list
  }
  stmt = alloc_statement(token->pos, STMT_DECLARATION);
  token = external_declaration(token, &stmt->declaration);
+ add_statement(list, stmt);
+ } else if (match_static_assert(token)) {
+ token = parse_static_assert(token, NULL);
  } else {
  seen_statement = Wdeclarationafterstatement;
  token = statement(token, &stmt);
+ add_statement(list, stmt);
  }
- add_statement(list, stmt);
  }
  return token;
 }

We don't need to relocate add_statment() into two places. We can add a
"continue"
after the static_assert() parsing.

> It doesn't appear to be possible to do a similar experiment with clang, in that
> case the "ISO C90 forbids mixed declarations and code" warning is only emitted
> with "-pedantic C90" or "-pedantic C89", and another warning is produced if
> _Static_assert() is used with those options:
>
>    warning: _Static_assert is a C11-specific feature [-Wc11-extensions]
>
> I would be happy to respin, taking the responses to the v5 patch as feedback.

That would be great. Looking forward to it. While you are there, can you make
it apply to master or sparse-next? I have update the master recently the V5
does not apply clean on master any more. It is trivial to fix though.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

From patchwork Thu May  4 16:00:38 2017
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [v5] sparse: add support for _Static_assert
From: Lance Richardson <[e-mail]>
X-Patchwork-Id: 9712267
Message-Id: <[e-mail]>
To: [e-mail]
Date: Thu,  4 May 2017 12:00:38 -0400

This patch introduces support for the C11 _Static_assert() construct.

Per the N1539 draft standard, the syntax changes for this construct
include:

    declaration:
        <declaration-specifiers> <init-declarator-list>[opt] ;
        <static_assert-declaration>

    struct-declaration:
        <specifier-qualifier-list> <struct-declarator-list>[opt] ;
        <static_assert-declaration>

    static_assert-declaration:
        _Static_assert ( <constant-expression> , <string-literal> ) ;

Signed-off-by: Lance Richardson <[e-mail]>
Acked-by: Luc Van Oostenryck <[e-mail]>
---
v5: Incorporated feedback from Christopher Li and Luc van Oostenryck:
    - Made _Static_assert a reserved identifier
    - Simplified check for _Static_assert keyword, consolidated into
      a common function.
    - Improved the "static assert within a function body" test case
      by adding a static assertion intermingled with code and adding
      a static assertion within a compound statement block.
    - Fixed use of initialized stmt variable.
    Tested by using sparse on entire kernel tree and a similarly-sized
    code tree which makes use of _Static_assert().

v4: Addressed feedback, simplified and restructured to better model
    description in draft standard.

v3:
    - Removed bogus test case introduced in v2 (static assertion on sizeof
      a structure within the definition of the structure).

v2: 
    - Added additional test cases.
    - Added additional validation for parameters to _Static_assert().
    - Reworked implementation to avoid impacting struct/union definition
      handling ( the v1 implementation, which treated _Static_assert()
      as an NS_TYPEDEF term, had the unfortunate side-effect of
      leaving an unnamed field with unknown size attached to structure
      definitions when a static assert was inside a structure definition).

 ident-list.h               |  1 +
 parse.c                    | 66 ++++++++++++++++++++++++++++++++++++++++------
 validation/static_assert.c | 64 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 123 insertions(+), 8 deletions(-)
 create mode 100644 validation/static_assert.c













--- sparse.chrisl.orig/parse.c
+++ sparse.chrisl/parse.c
@@ -73,6 +73,7 @@  static struct token *parse_context_state
 static struct token *parse_range_statement(struct token *token, struct statement *stmt);
 static struct token *parse_asm_statement(struct token *token, struct statement *stmt);
 static struct token *toplevel_asm_declaration(struct token *token, struct symbol_list **list);
+static struct token *parse_static_assert(struct token *token, struct symbol_list **unused);
 
 typedef struct token *attr_t(struct token *, struct symbol *,
 			     struct decl_state *);
@@ -328,6 +329,10 @@  static struct symbol_op asm_op = {
 	.toplevel = toplevel_asm_declaration,
 };
 
+static struct symbol_op static_assert_op = {
+	.toplevel = parse_static_assert,
+};
+
 static struct symbol_op packed_op = {
 	.attribute = attribute_packed,
 };
@@ -466,6 +471,9 @@  static struct init_keyword {
 	{ "__restrict",	NS_TYPEDEF, .op = &restrict_op},
 	{ "__restrict__",	NS_TYPEDEF, .op = &restrict_op},
 
+	/* Static assertion */
+	{ "_Static_assert", NS_KEYWORD, .op = &static_assert_op },
+
 	/* Storage class */
 	{ "auto",	NS_TYPEDEF, .op = &auto_op },
 	{ "register",	NS_TYPEDEF, .op = &register_op },
@@ -690,7 +698,6 @@  static int SENTINEL_ATTR match_idents(st
 	return next && token->ident == next;
 }
 
-
 struct statement *alloc_statement(struct position pos, int type)
 {
 	struct statement *stmt = __alloc_statement(0);
@@ -1942,11 +1949,18 @@  static struct token *declaration_list(st
 	return token;
 }
 
+static struct token *static_assert_or_declaration_list(struct token *token, struct symbol_list **list)
+{
+	if (match_ident(token, &_Static_assert_ident))
+		return parse_static_assert(token, NULL);
+	return declaration_list(token, list);
+}
+
 static struct token *struct_declaration_list(struct token *token, struct symbol_list **list)
 {
 	while (!match_op(token, '}')) {
 		if (!match_op(token, ';'))
-			token = declaration_list(token, list);
+			token = static_assert_or_declaration_list(token, list);
 		if (!match_op(token, ';')) {
 			sparse_error(token->pos, "expected ; at end of declaration");
 			break;
@@ -2093,6 +2107,40 @@  static struct token *parse_asm_declarato
 	return token;
 }
 
+
+static struct token *parse_static_assert(struct token *token, struct symbol_list **unused)
+{
+	struct expression *cond = NULL, *fail_string = NULL;
+	int val;
+
+	token = expect(token->next, '(', "after _Static_assert");
+	token = constant_expression(token, &cond);
+	if (!cond)
+		sparse_error(token->pos, "expect expression before '%s' token", show_token(token));
+	token = expect(token, ',', "after first argument of _Static_assert");
+	token = parse_expression(token, &fail_string);
+	if (!fail_string)
+		sparse_error(token->pos, "expect expression before '%s' token", show_token(token));
+	token = expect(token, ')', "after second argument of _Static_assert");
+
+	expect(token, ';', "after _Static_assert()");
+
+	if (!cond || !fail_string)
+		return token;
+
+	val = const_expression_value(cond);
+
+	if (fail_string->type != EXPR_STRING) {
+		sparse_error(fail_string->pos, "bad string literal");
+		return token;
+	}
+
+	if (cond->type == EXPR_VALUE && !val)
+		sparse_error(cond->pos, "static assertion failed: %s",
+				show_string(fail_string->string));
+	return token;
+}
+
 /* Make a statement out of an expression */
 static struct statement *make_statement(struct expression *expr)
 {
@@ -2474,13 +2522,18 @@  static struct token * statement_list(str
 			break;
 		if (match_op(token, '}'))
 			break;
-		if (lookup_type(token)) {
+		if (lookup_type(token) || match_ident(token, &_Static_assert_ident)) {
+			struct symbol_list *declaration = NULL;
+
 			if (seen_statement) {
 				warning(token->pos, "mixing declarations and code");
 				seen_statement = 0;
 			}
+			token = external_declaration(token, &declaration, NULL);
+			if (!declaration)
+				continue;
 			stmt = alloc_statement(token->pos, STMT_DECLARATION);
-			token = external_declaration(token, &stmt->declaration, NULL);
+			stmt->declaration = declaration;
 		} else {
 			seen_statement = Wdeclarationafterstatement;
 			token = statement(token, &stmt);
@@ -2819,7 +2872,7 @@  struct token *external_declaration(struc
 	unsigned long mod;
 	int is_typedef;
 
-	/* Top-level inline asm? */
+	/* Top-level inline asm or static assertion? */
 	if (token_type(token) == TOKEN_IDENT) {
 		struct symbol *s = lookup_keyword(token->ident, NS_KEYWORD);
 		if (s && s->op->toplevel)
--- sparse.chrisl.orig/ident-list.h
+++ sparse.chrisl/ident-list.h
@@ -16,6 +16,7 @@  IDENT_RESERVED(for);
 IDENT_RESERVED(while);
 IDENT_RESERVED(do);
 IDENT_RESERVED(goto);
+IDENT_RESERVED(_Static_assert);
 
 /* C typenames. They get marked as reserved when initialized */
 IDENT(struct);
--- sparse.chrisl.orig/validation/static_assert.c
+++ sparse.chrisl/validation/static_assert.c
@@ -0,0 +1,64 @@ 
+_Static_assert(1, "global ok");
+
+struct foo {
+	_Static_assert(1, "struct ok");
+};
+
+void bar(void)
+{
+	_Static_assert(1, " func1 ok");
+	int i;
+	i = 0;
+	_Static_assert(1, " func2 ok");
+
+        if (1) {
+	   _Static_assert(1, " func3 ok");
+	}
+}
+
+_Static_assert(0, "expected assertion failure");
+
+static int f;
+_Static_assert(f, "non-constant expression");
+
+static int *p;
+_Static_assert(p, "non-integer expression");
+
+_Static_assert(0.1, "float expression");
+        
+_Static_assert(!0 == 1, "non-trivial expression");
+        
+static char array[4];
+_Static_assert(sizeof(array) == 4, "sizeof expression");
+        
+static const char non_literal_string[] = "non literal string";
+_Static_assert(0, non_literal_string);
+
+_Static_assert(1 / 0, "invalid expression: should not show up?");
+
+struct s {
+	char arr[16];
+	_Static_assert(1, "inside struct");
+};
+
+union u {
+	char c;
+	int  i;
+	_Static_assert(1, "inside union");
+};
+
+_Static_assert(sizeof(struct s) == 16, "sizeof assertion");
+
+/*
+ * check-name: static assertion
+ *
+ * check-error-start
+static_assert.c:19:16: error: static assertion failed: "expected assertion failure"
+static_assert.c:22:16: error: bad constant expression
+static_assert.c:25:16: error: bad constant expression
+static_assert.c:27:16: error: bad constant expression
+static_assert.c:35:19: error: bad string literal
+static_assert.c:37:18: error: bad constant expression
+ * check-error-end
+ */
+