> From: "Christopher Li" <sparse@xxxxxxxxxxx>
> To: "Luc Van Oostenryck" <luc.vanoostenryck@xxxxxxxxx>
> Cc: "Lance Richardson" <lrichard@xxxxxxxxxx>, "Linux-Sparse" <linux-sparse@xxxxxxxxxxxxxxx>
> Sent: Sunday, October 30, 2016 12:43:32 PM
> Subject: Re: [PATCH v3] sparse: add support for static assert
>
> >+static struct symbol_op static_assert_op = {
> >+ .type = KW_ST_ASSERT,
> >+ .declarator = static_assert_declarator,
> >+ .statement = parse_static_assert_statement,
> >+ .toplevel = toplevel_static_assert,
> >+};
> >+
>
> There is no need to introduce the KW_ST_ASSERT. Will explain
> it later.
>
> static assert is not a declarator. It will introduce other
> issue if you treat it as one. For example, it allow the
> following illegal syntax: "_Static_assert(1, "") foo;"
>
> I think with ".statement" and ".toplevel" call back, it is sufficient
> to remove the ".declarator". Do I break any thing if I remove it?
>
>
> >+
> >+ if (token_type(token) == TOKEN_IDENT) {
> >+ keyword = lookup_keyword(token->ident, NS_KEYWORD);
> >+ if (keyword && keyword->op->type == KW_ST_ASSERT)
> >+ token = keyword->op->declarator(token,
> >NULL);
> >+ }
>
> There is much simpler way to do it. Please take a look at id-list.h.
>
> if (token_type(token) == TOKEN_IDENT && token->ident ==
> &_Static_assert_ident)
> ...
>
> That is why there is no need for the KW_ST_ASSERT.
>
> >+static struct token *parse_static_assert(struct token *token, int
> >expect_semi)
>
> I don't see which syntax can allow _Static_assert() not following the
> semi column.
> Do you have an example?
>
> I can see const_expression_value will expand the expression as needed.
> I am still wondering if there is any thing else need to process at
> a later stage like expanding.
>
> The test example is pretty good. We can use a little bit more test on
> the path that trigger the assert.
>
> Thanks for the patch and sorry for the late replay.
>
> Chris
>
Thanks for reviewing, Chris, I will work on a v4 to incorporate your feedback.
Lance
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
