On Sun, Jan 10, 2016 at 09:54:24PM -0500, Lance Richardson wrote: > ----- Original Message ----- > > On Wed, Jan 06, 2016 at 01:35:17PM -0500, Lance Richardson wrote: > > > This patch introduces support for _Static_assert() in global, > > > function, and struct/union declaration contexts (as currently supported > > > by gcc). > > > > > > Tested via: > > > - kernel build with C=1 CF=-D__CHECK_ENDIAN__ > > > - build/check large code base making heavy use of _Static_assert() > > > - "make check" with added test case for static assert support > > > > Nice, it was something that was indeed missing. > > > > I have a few remarks, mostly nitpicking, here under. > > Hi Luc, thanks for taking the time to review this. Glad to help! ... > > > > I find also a bit strange to have this sort of semantic check inside the > > parser. > > This prompted me to look at the gcc implementation, it appears that gcc also > processes _Static_assert() during parsing. It can always be moved elsewhere if needed. > > Also, what should happen with: > > _Static_assert(1 / 0, "invalid expression: should not show up?"); > > (The C11 standard is not clear to me, GCC outputs an "invalid expression" > > message and ignore the assertion). > > v2 will output something like "invalid constant integer expression". Please don't. That's the job of constant_expression() to emit a good error message. Here, the best thing to do is to *not* emit an error message if the expression is not a valid expression, constant (for example, by forcing val to 1 when expr is null). > > > > Finally, the standard also allow to place the assertion > > inside a struct or union declaration; like: > > struct s { > > char c; > > _Static_assert(1, "inside struct"); > > }; > > which is working fine. > > And also, I think: > > struct s2 { > > char c; > > _Static_assert(sizeof(struct s) == 1, "own struct sizeof"); > > }; > > which doesn't. > > The second case should work (unless you intended "sizeof(struct s2)"), the v2 > implementation will be a little different to address this issue. Yep, I messed it up. I indeed meant "sizeof(struct s2)" and it seems to work fine but for the wrong reason. See what I mean with the following example: struct s3 { char c; _Static_assert(sizeof(struct s3) == 1, "own struct sizeof, partial 1"); _Static_assert(sizeof(struct s3) == 2, "own struct sizeof, partial 2"); char d; }; but this problem is unrelated to your patch. Yours, Luc -- 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