Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
On 12/13/2012 03:43 AM, Jan Beulich wrote: On 13.12.12 at 01:29, Daniel Santos wrote: Wow, it's really easy to miss parallel development on the same issue. Sorry for my late response to this thread. I started another thread addressing these issues (as well as a few others) back in September (https://lkml.org/lkml/2012/9/28/1136). I've finally gotten ACKs from maintainers with v6 of the patches (here https://lkml.org/lkml/2012/11/20/621) and I'm just waiting for 3.8-rc1 to re-submit them. I actually submitted these patches back in June as part of a larger patch set, but broke it apart in September (I had way to many changes for one patch set) Since yours is apparently ready to go in, but doesn't use _Static_assert, I guess I'll wait for it to appear until I re-work whatever might be left to actually make use of _Static_assert. Jan Interesting! They've enabled it by default (I suppose as an extension?) in every standard (except -pedantic of course). One minor draw-back is that it appears to enjoy escaping tickmarks in the error message. I've opened a bug for it (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55678) But realistically, the "compiletime_assert" macro I wrote in compiler.h can be renamed to "static_assert", analogous to C11's static_assert from assert.h (§7.2 of C11) and it can expand to the _Static_assert keyword, when that is available. Something else that I didn't consider too much before was support for compiling -O0 or -O1, which will cause many expressions that are otherwise evaluated as compile-time constants to become non-constant and result in failed assertions. This isn't anything new however, building -O0 has been broken for quite some time, but I presume it could help some development of out of tree modules. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
>>> On 13.12.12 at 01:29, Daniel Santos wrote: > Wow, it's really easy to miss parallel development on the same issue. > Sorry for my late response to this thread. I started another thread > addressing these issues (as well as a few others) back in September > (https://lkml.org/lkml/2012/9/28/1136). I've finally gotten ACKs from > maintainers with v6 of the patches (here > https://lkml.org/lkml/2012/11/20/621) and I'm just waiting for 3.8-rc1 > to re-submit them. I actually submitted these patches back in June as > part of a larger patch set, but broke it apart in September (I had way > to many changes for one patch set) Since yours is apparently ready to go in, but doesn't use _Static_assert, I guess I'll wait for it to appear until I re-work whatever might be left to actually make use of _Static_assert. Jan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
On 13.12.12 at 01:29, Daniel Santos danielfsan...@att.net wrote: Wow, it's really easy to miss parallel development on the same issue. Sorry for my late response to this thread. I started another thread addressing these issues (as well as a few others) back in September (https://lkml.org/lkml/2012/9/28/1136). I've finally gotten ACKs from maintainers with v6 of the patches (here https://lkml.org/lkml/2012/11/20/621) and I'm just waiting for 3.8-rc1 to re-submit them. I actually submitted these patches back in June as part of a larger patch set, but broke it apart in September (I had way to many changes for one patch set) Since yours is apparently ready to go in, but doesn't use _Static_assert, I guess I'll wait for it to appear until I re-work whatever might be left to actually make use of _Static_assert. Jan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
On 12/13/2012 03:43 AM, Jan Beulich wrote: On 13.12.12 at 01:29, Daniel Santosdanielfsan...@att.net wrote: Wow, it's really easy to miss parallel development on the same issue. Sorry for my late response to this thread. I started another thread addressing these issues (as well as a few others) back in September (https://lkml.org/lkml/2012/9/28/1136). I've finally gotten ACKs from maintainers with v6 of the patches (here https://lkml.org/lkml/2012/11/20/621) and I'm just waiting for 3.8-rc1 to re-submit them. I actually submitted these patches back in June as part of a larger patch set, but broke it apart in September (I had way to many changes for one patch set) Since yours is apparently ready to go in, but doesn't use _Static_assert, I guess I'll wait for it to appear until I re-work whatever might be left to actually make use of _Static_assert. Jan Interesting! They've enabled it by default (I suppose as an extension?) in every standard (except -pedantic of course). One minor draw-back is that it appears to enjoy escaping tickmarks in the error message. I've opened a bug for it (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55678) But realistically, the compiletime_assert macro I wrote in compiler.h can be renamed to static_assert, analogous to C11's static_assert from assert.h (§7.2 of C11) and it can expand to the _Static_assert keyword, when that is available. Something else that I didn't consider too much before was support for compiling -O0 or -O1, which will cause many expressions that are otherwise evaluated as compile-time constants to become non-constant and result in failed assertions. This isn't anything new however, building -O0 has been broken for quite some time, but I presume it could help some development of out of tree modules. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
On 11/06/2012 03:23 AM, Jan Beulich wrote:
This sort of logic is normally performed via the
include/linux/compiler*.h system. And
grep __GNUC include/linux/*.h
indicates that we've been pretty successful. Can we do that here too?
(eg: suppose the Intel compiler supports _Static_assert?)
Yeah, there are a lot of goodies here:
_Static_assert:
We could define __ASSERT_STRUCT_FIELD(e) for this:
#define BUILD_BUG_ON_ZERO(e) \
sizeof(struct { __ASSERT_STRUCT_FIELD(e); })
I considered something like this too, but it wouldn't work well: The
diagnostic issued from a failed _Static_assert() would preferably
refer to the original source expression, not an already macro
expanded variant of it. That, however, precludes passing it
through multiple levels of macro expansion. Which would leave
passing e and #e to __ASSERT_STRUCT_FIELD(), but that's again
ugly as it opens the door for someone adding a use where the two
arguments don't match up.
I was under the assumption that _Static_assert doesn't exist unless you
are using -std=c11 or -std=gnu11, but I admit I didn't look into it
much. I'm very glad it was added to the standard. IMO, it would be
better to use such a feature (presuming it behaves well) when it is
available but, of course, we still need the best possible functionality
when it isn't.
In either case, the new BUILD_BUG_ON macro simply stringifies the
condition, so it will emit an error indicating the exact expression that
failed. If you pass it a macro, macro expansion will not be performed
prior to stringification.
__compiletime_error():
I blame Arjan for this. It disappears if not implemented, which
is just lazy. BUILD_BUG_ON() does this right, and he didn't fix
that at the time :(
Again, the name of the macro made me not use it, as the obvious
fallback is a link time error. The only thing I would be agreeable to
is something like __buildtime_error().
Yeah, I agree, since we speak more in terms of a "build" rather than
compiling. But I didn't mess with it in my patches.
I'd say we have three patches here, really:
1) Add __ASSERT_STRUCT_FIELD(e) to compiler.h
There are actually a number of reasons for having multiple mechanisms to
break the build. For instance, BUILD_BUG_ON_ZERO is used outside of
function bodies, where a compound gcc expression ({expr; expr;}) isn't
permitted.
2) Add __UNIQUE_ID().
3) Use them (I can think of at least one other place for __UNIQUE_ID()).
Jan, do you want the glory? :) If not, I'll respin.
Depending on the answers to the above (i.e. how much of it
actually would need re-doing and re-testing), it may take me
some time to get to this, but beyond that I'm fine with trying
to improve this to everyone's satisfaction.
Personally, I would rather we start a cpputil.h (or some such) to
encapsulate all of our preprocessor kludgery, err, I mean wizardry.
There must be 14 thousand paste macros in the kernel. We could move
stringify into it as well, but I suppose that's another issue.
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
Wow, it's really easy to miss parallel development on the same issue.
Sorry for my late response to this thread. I started another thread
addressing these issues (as well as a few others) back in September
(https://lkml.org/lkml/2012/9/28/1136). I've finally gotten ACKs from
maintainers with v6 of the patches (here
https://lkml.org/lkml/2012/11/20/621) and I'm just waiting for 3.8-rc1
to re-submit them. I actually submitted these patches back in June as
part of a larger patch set, but broke it apart in September (I had way
to many changes for one patch set)
On 11/07/2012 05:24 PM, Rusty Russell wrote:
Jan Beulich writes:
On 07.11.12 at 02:03, Rusty Russell wrote:
__COUNTER__:
Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
this (using __COUNTER__ or __LINE__). 4.3 and above.
The fallback to __LINE__ is not necessarily correct in all cases,
namely when deep macro expansions result in two instances used
on the same source line, or a use in a header matches line number
wise a second use in another header or the source file.
I considered to option of a fallback (and hence an abstraction in
compiler*.h) when doing this, but I didn't want to do something
that would break in perhaps vary obscure ways.
I was thinking of my own code in moduleparam.h, but elfnote.h does the
same trick. In both cases, it'll simply fail compile if we fallback and
__LINE__ isn't unique.
So again, I don't think we're going to see many uses, and no existing
use will bite us.
That's exactly the point - we can't know (because there's no
guarantee there aren't - or won't be by the time it might get
merged - any two conflicting uses of BUILD_BUG_ON...().
Conflicting BUILD_BUG_ON() is completely harmless: two identical
undefines are OK. The other cases cause a compile error, and one which
is trivial to fix, and I'm happy to fix it if it does.
I've never seen a construction in the kernel which would silently break
if __UNIQUE_ID() isn't actually unique. That makes sense, because you
if the symbol wasn't exposed you wouldn't need a __UNIQUE_ID. And if
it's exposed, the compiler will give an error on multiple definition.
(Obviously you can't have non-static __UNIQUE_ID() because it's only
ever unique across a single gcc compile).
I was considering __COUNTER__, but in the end, others felt that the
inconsistency could cause problems later. However, with the
__compiletime_error function attribute, the result will only be that the
actual error message emitted will be incorrect, since the first error
attribute assigned to the extern function will be used. Thus, compiling
this code:
{extern void func(void) __attribute__((error("hello")));}
{extern void func(void) __attribute__((error("goodbye"))); func();}
would result in an error with the message "hello".
#define __build_bug_on_failed(n) __build_bug_on_##n##_failed
#define _BUILD_BUG_ON(n, condition) \
do {\
extern void __compiletime_error(#condition) \
__build_bug_on_failed(n)(void); \
if (condition) __build_bug_on_failed(n)(); \
} while(0)
Actually, I just noticed that __linktime_error() really is a compile
time error when gcc supports __attribute__(__error__()), so
probably using that one instead of __compiletime_error() here
would be the cleaner approach.
The one thing puzzling me here is that __linktime_error() gets
defined even if __CHECKER__ isn't defined, while
__compiletime_error() doesn't - an apparent inconsistency
between 746a2a838deec3ef86ef6b7c3edd4207b9a351aa
(Kosaki?) and 1399ff86f2a2bbacbbe68fa00c5f8c752b344723
(David?), with apparently the latter being too lax, as it only
introduced a use inside a !__CHECKER__ section.
Yes, __linktime_error() makes no sense, since it's identical to
__compiletime_error(). It's also a terrible name. Let's kill it.
Definitely. My patch set kills that sucker too.
I hadn't noticed BUILD_BUG. We should fix that as discussed here, and
simply make BUILD_BUG_ON() call it.
Subject: __UNIQUE_ID()
Jan Beulich points out __COUNTER__ (gcc 4.3 and above), so let's use
that to create unique ids. This is better than __LINE__ which we use
today, so provide a wrapper.
Signed-off-by: Rusty Russell
Acked-by: Jan Beulich
Thanks,
Rusty.
https://lkml.org/lkml/2012/9/28/1136
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
Wow, it's really easy to miss parallel development on the same issue.
Sorry for my late response to this thread. I started another thread
addressing these issues (as well as a few others) back in September
(https://lkml.org/lkml/2012/9/28/1136). I've finally gotten ACKs from
maintainers with v6 of the patches (here
https://lkml.org/lkml/2012/11/20/621) and I'm just waiting for 3.8-rc1
to re-submit them. I actually submitted these patches back in June as
part of a larger patch set, but broke it apart in September (I had way
to many changes for one patch set)
On 11/07/2012 05:24 PM, Rusty Russell wrote:
Jan Beulichjbeul...@suse.com writes:
On 07.11.12 at 02:03, Rusty Russellru...@rustcorp.com.au wrote:
__COUNTER__:
Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
this (using __COUNTER__ or __LINE__). 4.3 and above.
The fallback to __LINE__ is not necessarily correct in all cases,
namely when deep macro expansions result in two instances used
on the same source line, or a use in a header matches line number
wise a second use in another header or the source file.
I considered to option of a fallback (and hence an abstraction in
compiler*.h) when doing this, but I didn't want to do something
that would break in perhaps vary obscure ways.
I was thinking of my own code in moduleparam.h, but elfnote.h does the
same trick. In both cases, it'll simply fail compile if we fallback and
__LINE__ isn't unique.
So again, I don't think we're going to see many uses, and no existing
use will bite us.
That's exactly the point - we can't know (because there's no
guarantee there aren't - or won't be by the time it might get
merged - any two conflicting uses of BUILD_BUG_ON...().
Conflicting BUILD_BUG_ON() is completely harmless: two identical
undefines are OK. The other cases cause a compile error, and one which
is trivial to fix, and I'm happy to fix it if it does.
I've never seen a construction in the kernel which would silently break
if __UNIQUE_ID() isn't actually unique. That makes sense, because you
if the symbol wasn't exposed you wouldn't need a __UNIQUE_ID. And if
it's exposed, the compiler will give an error on multiple definition.
(Obviously you can't have non-static __UNIQUE_ID() because it's only
ever unique across a single gcc compile).
I was considering __COUNTER__, but in the end, others felt that the
inconsistency could cause problems later. However, with the
__compiletime_error function attribute, the result will only be that the
actual error message emitted will be incorrect, since the first error
attribute assigned to the extern function will be used. Thus, compiling
this code:
{extern void func(void) __attribute__((error(hello)));}
{extern void func(void) __attribute__((error(goodbye))); func();}
would result in an error with the message hello.
#define __build_bug_on_failed(n) __build_bug_on_##n##_failed
#define _BUILD_BUG_ON(n, condition) \
do {\
extern void __compiletime_error(#condition) \
__build_bug_on_failed(n)(void); \
if (condition) __build_bug_on_failed(n)(); \
} while(0)
Actually, I just noticed that __linktime_error() really is a compile
time error when gcc supports __attribute__(__error__()), so
probably using that one instead of __compiletime_error() here
would be the cleaner approach.
The one thing puzzling me here is that __linktime_error() gets
defined even if __CHECKER__ isn't defined, while
__compiletime_error() doesn't - an apparent inconsistency
between 746a2a838deec3ef86ef6b7c3edd4207b9a351aa
(Kosaki?) and 1399ff86f2a2bbacbbe68fa00c5f8c752b344723
(David?), with apparently the latter being too lax, as it only
introduced a use inside a !__CHECKER__ section.
Yes, __linktime_error() makes no sense, since it's identical to
__compiletime_error(). It's also a terrible name. Let's kill it.
Definitely. My patch set kills that sucker too.
I hadn't noticed BUILD_BUG. We should fix that as discussed here, and
simply make BUILD_BUG_ON() call it.
Subject: __UNIQUE_ID()
Jan Beulich points out __COUNTER__ (gcc 4.3 and above), so let's use
that to create unique ids. This is better than __LINE__ which we use
today, so provide a wrapper.
Signed-off-by: Rusty Russellru...@rustcorp.com.au
Acked-by: Jan Beulichjbeul...@suse.com
Thanks,
Rusty.
https://lkml.org/lkml/2012/9/28/1136
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
On 11/06/2012 03:23 AM, Jan Beulich wrote:
This sort of logic is normally performed via the
include/linux/compiler*.h system. And
grep __GNUC include/linux/*.h
indicates that we've been pretty successful. Can we do that here too?
(eg: suppose the Intel compiler supports _Static_assert?)
Yeah, there are a lot of goodies here:
_Static_assert:
We could define __ASSERT_STRUCT_FIELD(e) for this:
#define BUILD_BUG_ON_ZERO(e) \
sizeof(struct { __ASSERT_STRUCT_FIELD(e); })
I considered something like this too, but it wouldn't work well: The
diagnostic issued from a failed _Static_assert() would preferably
refer to the original source expression, not an already macro
expanded variant of it. That, however, precludes passing it
through multiple levels of macro expansion. Which would leave
passing e and #e to __ASSERT_STRUCT_FIELD(), but that's again
ugly as it opens the door for someone adding a use where the two
arguments don't match up.
I was under the assumption that _Static_assert doesn't exist unless you
are using -std=c11 or -std=gnu11, but I admit I didn't look into it
much. I'm very glad it was added to the standard. IMO, it would be
better to use such a feature (presuming it behaves well) when it is
available but, of course, we still need the best possible functionality
when it isn't.
In either case, the new BUILD_BUG_ON macro simply stringifies the
condition, so it will emit an error indicating the exact expression that
failed. If you pass it a macro, macro expansion will not be performed
prior to stringification.
__compiletime_error():
I blame Arjan for this. It disappears if not implemented, which
is just lazy. BUILD_BUG_ON() does this right, and he didn't fix
that at the time :(
Again, the name of the macro made me not use it, as the obvious
fallback is a link time error. The only thing I would be agreeable to
is something like __buildtime_error().
Yeah, I agree, since we speak more in terms of a build rather than
compiling. But I didn't mess with it in my patches.
I'd say we have three patches here, really:
1) Add __ASSERT_STRUCT_FIELD(e) to compiler.h
There are actually a number of reasons for having multiple mechanisms to
break the build. For instance, BUILD_BUG_ON_ZERO is used outside of
function bodies, where a compound gcc expression ({expr; expr;}) isn't
permitted.
2) Add __UNIQUE_ID().
3) Use them (I can think of at least one other place for __UNIQUE_ID()).
Jan, do you want the glory? :) If not, I'll respin.
Depending on the answers to the above (i.e. how much of it
actually would need re-doing and re-testing), it may take me
some time to get to this, but beyond that I'm fine with trying
to improve this to everyone's satisfaction.
Personally, I would rather we start a cpputil.h (or some such) to
encapsulate all of our preprocessor kludgery, err, I mean wizardry.
There must be 14 thousand paste macros in the kernel. We could move
stringify into it as well, but I suppose that's another issue.
Daniel
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
Jan Beulich writes:
On 07.11.12 at 02:03, Rusty Russell wrote:
__COUNTER__:
Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
this (using __COUNTER__ or __LINE__). 4.3 and above.
>>>
>>> The fallback to __LINE__ is not necessarily correct in all cases,
>>> namely when deep macro expansions result in two instances used
>>> on the same source line, or a use in a header matches line number
>>> wise a second use in another header or the source file.
>>>
>>> I considered to option of a fallback (and hence an abstraction in
>>> compiler*.h) when doing this, but I didn't want to do something
>>> that would break in perhaps vary obscure ways.
>>
>> I was thinking of my own code in moduleparam.h, but elfnote.h does the
>> same trick. In both cases, it'll simply fail compile if we fallback and
>> __LINE__ isn't unique.
>>
>> So again, I don't think we're going to see many uses, and no existing
>> use will bite us.
>
> That's exactly the point - we can't know (because there's no
> guarantee there aren't - or won't be by the time it might get
> merged - any two conflicting uses of BUILD_BUG_ON...().
Conflicting BUILD_BUG_ON() is completely harmless: two identical
undefines are OK. The other cases cause a compile error, and one which
is trivial to fix, and I'm happy to fix it if it does.
I've never seen a construction in the kernel which would silently break
if __UNIQUE_ID() isn't actually unique. That makes sense, because you
if the symbol wasn't exposed you wouldn't need a __UNIQUE_ID. And if
it's exposed, the compiler will give an error on multiple definition.
(Obviously you can't have non-static __UNIQUE_ID() because it's only
ever unique across a single gcc compile).
>>> #define __build_bug_on_failed(n) __build_bug_on_##n##_failed
>>> #define _BUILD_BUG_ON(n, condition) \
>>> do {\
>>> extern void __compiletime_error(#condition) \
>>> __build_bug_on_failed(n)(void); \
>>> if (condition) __build_bug_on_failed(n)(); \
>>> } while(0)
>
> Actually, I just noticed that __linktime_error() really is a compile
> time error when gcc supports __attribute__(__error__()), so
> probably using that one instead of __compiletime_error() here
> would be the cleaner approach.
>
> The one thing puzzling me here is that __linktime_error() gets
> defined even if __CHECKER__ isn't defined, while
> __compiletime_error() doesn't - an apparent inconsistency
> between 746a2a838deec3ef86ef6b7c3edd4207b9a351aa
> (Kosaki?) and 1399ff86f2a2bbacbbe68fa00c5f8c752b344723
> (David?), with apparently the latter being too lax, as it only
> introduced a use inside a !__CHECKER__ section.
Yes, __linktime_error() makes no sense, since it's identical to
__compiletime_error(). It's also a terrible name. Let's kill it.
I hadn't noticed BUILD_BUG. We should fix that as discussed here, and
simply make BUILD_BUG_ON() call it.
>> Subject: __UNIQUE_ID()
>>
>> Jan Beulich points out __COUNTER__ (gcc 4.3 and above), so let's use
>> that to create unique ids. This is better than __LINE__ which we use
>> today, so provide a wrapper.
>>
>> Signed-off-by: Rusty Russell
>
> Acked-by: Jan Beulich
Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
>>> On 07.11.12 at 02:03, Rusty Russell wrote:
> Jan Beulich writes:
> On 06.11.12 at 02:51, Rusty Russell wrote:
>>> Yeah, there are a lot of goodies here:
>>>
>>> _Static_assert:
>>> We could define __ASSERT_STRUCT_FIELD(e) for this:
>>> #define BUILD_BUG_ON_ZERO(e) \
>>> sizeof(struct { __ASSERT_STRUCT_FIELD(e); })
>>
>> I considered something like this too, but it wouldn't work well: The
>> diagnostic issued from a failed _Static_assert() would preferably
>> refer to the original source expression, not an already macro
>> expanded variant of it. That, however, precludes passing it
>> through multiple levels of macro expansion. Which would leave
>> passing e and #e to __ASSERT_STRUCT_FIELD(), but that's again
>> ugly as it opens the door for someone adding a use where the two
>> arguments don't match up.
>
> Good point, but this is going to be pretty damn obscure. In fact, I
> don't think it'll see more than the use here.
Okay, so I'll go with something like that then.
>>> __COUNTER__:
>>> Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
>>> this (using __COUNTER__ or __LINE__). 4.3 and above.
>>
>> The fallback to __LINE__ is not necessarily correct in all cases,
>> namely when deep macro expansions result in two instances used
>> on the same source line, or a use in a header matches line number
>> wise a second use in another header or the source file.
>>
>> I considered to option of a fallback (and hence an abstraction in
>> compiler*.h) when doing this, but I didn't want to do something
>> that would break in perhaps vary obscure ways.
>
> I was thinking of my own code in moduleparam.h, but elfnote.h does the
> same trick. In both cases, it'll simply fail compile if we fallback and
> __LINE__ isn't unique.
>
> So again, I don't think we're going to see many uses, and no existing
> use will bite us.
That's exactly the point - we can't know (because there's no
guarantee there aren't - or won't be by the time it might get
merged - any two conflicting uses of BUILD_BUG_ON...().
>>> __compiletime_error():
>>> I blame Arjan for this. It disappears if not implemented, which
>>> is just lazy. BUILD_BUG_ON() does this right, and he didn't fix
>>> that at the time :(
>>
>> Again, the name of the macro made me not use it, as the obvious
>> fallback is a link time error. The only thing I would be agreeable to
>> is something like __buildtime_error().
>
> Yes, it's awkward to use. Your own usage here is the only correct way
> to do it, AFAICT:
>
>> #define __build_bug_on_failed(n) __build_bug_on_##n##_failed
>> #define _BUILD_BUG_ON(n, condition) \
>> do {\
>> extern void __compiletime_error(#condition) \
>> __build_bug_on_failed(n)(void); \
>> if (condition) __build_bug_on_failed(n)(); \
>> } while(0)
Actually, I just noticed that __linktime_error() really is a compile
time error when gcc supports __attribute__(__error__()), so
probably using that one instead of __compiletime_error() here
would be the cleaner approach.
The one thing puzzling me here is that __linktime_error() gets
defined even if __CHECKER__ isn't defined, while
__compiletime_error() doesn't - an apparent inconsistency
between 746a2a838deec3ef86ef6b7c3edd4207b9a351aa
(Kosaki?) and 1399ff86f2a2bbacbbe68fa00c5f8c752b344723
(David?), with apparently the latter being too lax, as it only
introduced a use inside a !__CHECKER__ section.
>> #define BUILD_BUG_ON(condition...) _BUILD_BUG_ON(__COUNTER__, ##condition)
>
> The ... is overkill here: sure it's general, but we don't allow that
> currently, nor in the other BUILD_BUG_ON() definitions.
This may be a leftover from previous failed attempts; I don't
see a reason why it should stay.
> So I think this becomes:
>
> #define _BUILD_BUG_ON(undefname, condition)
> \
> do {
> \
> extern void __compiletime_error(#condition) undefname(void);
> \
> if (condition) undefname();
> \
> } while(0)
> #define BUILD_BUG_ON(condition) \
> _BUILD_BUG_ON(__UNIQUE_ID(__build_bug_fail), (condition))
Yes, subject to the fallback issue.
> Subject: __UNIQUE_ID()
>
> Jan Beulich points out __COUNTER__ (gcc 4.3 and above), so let's use
> that to create unique ids. This is better than __LINE__ which we use
> today, so provide a wrapper.
>
> Signed-off-by: Rusty Russell
Acked-by: Jan Beulich
> diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
> index 412bc6c..8908821 100644
> --- a/include/linux/compiler-gcc4.h
> +++ b/include/linux/compiler-gcc4.h
> @@ -31,6 +31,8 @@
>
> #define __linktime_error(message)
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
On 07.11.12 at 02:03, Rusty Russell ru...@rustcorp.com.au wrote:
Jan Beulich jbeul...@suse.com writes:
On 06.11.12 at 02:51, Rusty Russell ru...@rustcorp.com.au wrote:
Yeah, there are a lot of goodies here:
_Static_assert:
We could define __ASSERT_STRUCT_FIELD(e) for this:
#define BUILD_BUG_ON_ZERO(e) \
sizeof(struct { __ASSERT_STRUCT_FIELD(e); })
I considered something like this too, but it wouldn't work well: The
diagnostic issued from a failed _Static_assert() would preferably
refer to the original source expression, not an already macro
expanded variant of it. That, however, precludes passing it
through multiple levels of macro expansion. Which would leave
passing e and #e to __ASSERT_STRUCT_FIELD(), but that's again
ugly as it opens the door for someone adding a use where the two
arguments don't match up.
Good point, but this is going to be pretty damn obscure. In fact, I
don't think it'll see more than the use here.
Okay, so I'll go with something like that then.
__COUNTER__:
Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
this (using __COUNTER__ or __LINE__). 4.3 and above.
The fallback to __LINE__ is not necessarily correct in all cases,
namely when deep macro expansions result in two instances used
on the same source line, or a use in a header matches line number
wise a second use in another header or the source file.
I considered to option of a fallback (and hence an abstraction in
compiler*.h) when doing this, but I didn't want to do something
that would break in perhaps vary obscure ways.
I was thinking of my own code in moduleparam.h, but elfnote.h does the
same trick. In both cases, it'll simply fail compile if we fallback and
__LINE__ isn't unique.
So again, I don't think we're going to see many uses, and no existing
use will bite us.
That's exactly the point - we can't know (because there's no
guarantee there aren't - or won't be by the time it might get
merged - any two conflicting uses of BUILD_BUG_ON...().
__compiletime_error():
I blame Arjan for this. It disappears if not implemented, which
is just lazy. BUILD_BUG_ON() does this right, and he didn't fix
that at the time :(
Again, the name of the macro made me not use it, as the obvious
fallback is a link time error. The only thing I would be agreeable to
is something like __buildtime_error().
Yes, it's awkward to use. Your own usage here is the only correct way
to do it, AFAICT:
#define __build_bug_on_failed(n) __build_bug_on_##n##_failed
#define _BUILD_BUG_ON(n, condition) \
do {\
extern void __compiletime_error(#condition) \
__build_bug_on_failed(n)(void); \
if (condition) __build_bug_on_failed(n)(); \
} while(0)
Actually, I just noticed that __linktime_error() really is a compile
time error when gcc supports __attribute__(__error__()), so
probably using that one instead of __compiletime_error() here
would be the cleaner approach.
The one thing puzzling me here is that __linktime_error() gets
defined even if __CHECKER__ isn't defined, while
__compiletime_error() doesn't - an apparent inconsistency
between 746a2a838deec3ef86ef6b7c3edd4207b9a351aa
(Kosaki?) and 1399ff86f2a2bbacbbe68fa00c5f8c752b344723
(David?), with apparently the latter being too lax, as it only
introduced a use inside a !__CHECKER__ section.
#define BUILD_BUG_ON(condition...) _BUILD_BUG_ON(__COUNTER__, ##condition)
The ... is overkill here: sure it's general, but we don't allow that
currently, nor in the other BUILD_BUG_ON() definitions.
This may be a leftover from previous failed attempts; I don't
see a reason why it should stay.
So I think this becomes:
#define _BUILD_BUG_ON(undefname, condition)
\
do {
\
extern void __compiletime_error(#condition) undefname(void);
\
if (condition) undefname();
\
} while(0)
#define BUILD_BUG_ON(condition) \
_BUILD_BUG_ON(__UNIQUE_ID(__build_bug_fail), (condition))
Yes, subject to the fallback issue.
Subject: __UNIQUE_ID()
Jan Beulich points out __COUNTER__ (gcc 4.3 and above), so let's use
that to create unique ids. This is better than __LINE__ which we use
today, so provide a wrapper.
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Acked-by: Jan Beulich jbeul...@suse.com
diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 412bc6c..8908821 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -31,6 +31,8 @@
#define __linktime_error(message) __attribute__((__error__(message)))
+#define __UNIQUE_ID(prefix) __PASTE(prefix,
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
Jan Beulich jbeul...@suse.com writes:
On 07.11.12 at 02:03, Rusty Russell ru...@rustcorp.com.au wrote:
__COUNTER__:
Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
this (using __COUNTER__ or __LINE__). 4.3 and above.
The fallback to __LINE__ is not necessarily correct in all cases,
namely when deep macro expansions result in two instances used
on the same source line, or a use in a header matches line number
wise a second use in another header or the source file.
I considered to option of a fallback (and hence an abstraction in
compiler*.h) when doing this, but I didn't want to do something
that would break in perhaps vary obscure ways.
I was thinking of my own code in moduleparam.h, but elfnote.h does the
same trick. In both cases, it'll simply fail compile if we fallback and
__LINE__ isn't unique.
So again, I don't think we're going to see many uses, and no existing
use will bite us.
That's exactly the point - we can't know (because there's no
guarantee there aren't - or won't be by the time it might get
merged - any two conflicting uses of BUILD_BUG_ON...().
Conflicting BUILD_BUG_ON() is completely harmless: two identical
undefines are OK. The other cases cause a compile error, and one which
is trivial to fix, and I'm happy to fix it if it does.
I've never seen a construction in the kernel which would silently break
if __UNIQUE_ID() isn't actually unique. That makes sense, because you
if the symbol wasn't exposed you wouldn't need a __UNIQUE_ID. And if
it's exposed, the compiler will give an error on multiple definition.
(Obviously you can't have non-static __UNIQUE_ID() because it's only
ever unique across a single gcc compile).
#define __build_bug_on_failed(n) __build_bug_on_##n##_failed
#define _BUILD_BUG_ON(n, condition) \
do {\
extern void __compiletime_error(#condition) \
__build_bug_on_failed(n)(void); \
if (condition) __build_bug_on_failed(n)(); \
} while(0)
Actually, I just noticed that __linktime_error() really is a compile
time error when gcc supports __attribute__(__error__()), so
probably using that one instead of __compiletime_error() here
would be the cleaner approach.
The one thing puzzling me here is that __linktime_error() gets
defined even if __CHECKER__ isn't defined, while
__compiletime_error() doesn't - an apparent inconsistency
between 746a2a838deec3ef86ef6b7c3edd4207b9a351aa
(Kosaki?) and 1399ff86f2a2bbacbbe68fa00c5f8c752b344723
(David?), with apparently the latter being too lax, as it only
introduced a use inside a !__CHECKER__ section.
Yes, __linktime_error() makes no sense, since it's identical to
__compiletime_error(). It's also a terrible name. Let's kill it.
I hadn't noticed BUILD_BUG. We should fix that as discussed here, and
simply make BUILD_BUG_ON() call it.
Subject: __UNIQUE_ID()
Jan Beulich points out __COUNTER__ (gcc 4.3 and above), so let's use
that to create unique ids. This is better than __LINE__ which we use
today, so provide a wrapper.
Signed-off-by: Rusty Russell ru...@rustcorp.com.au
Acked-by: Jan Beulich jbeul...@suse.com
Thanks,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
Jan Beulich writes:
On 06.11.12 at 02:51, Rusty Russell wrote:
>> Andrew Morton writes:
>>
>>> On Fri, 02 Nov 2012 14:47:40 +
>>> "Jan Beulich" wrote:
>>>
This makes the resulting diagnostics quite a bit more useful.
>>>
>>> So asserts Jan, but to confirm it I would need to download, configure,
>>> build and install a different gcc version, which sounds rather a hassle.
>>>
>>> So, please show us an exmple of these diagnostics in the changelog.
>>>
--- 3.7-rc3/include/linux/bug.h
+++ 3.7-rc3-static-assert/include/linux/bug.h
@@ -27,8 +27,15 @@ struct pt_regs;
result (of value 0 and type size_t), so the expression can be used
e.g. in a structure initializer (or where-ever else comma expressions
aren't permitted). */
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
>>>
>>> This sort of logic is normally performed via the
>>> include/linux/compiler*.h system. And
>>>
>>> grep __GNUC include/linux/*.h
>>>
>>> indicates that we've been pretty successful. Can we do that here too?
>>>
>>> (eg: suppose the Intel compiler supports _Static_assert?)
>>
>> Yeah, there are a lot of goodies here:
>>
>> _Static_assert:
>> We could define __ASSERT_STRUCT_FIELD(e) for this:
>> #define BUILD_BUG_ON_ZERO(e) \
>> sizeof(struct { __ASSERT_STRUCT_FIELD(e); })
>
> I considered something like this too, but it wouldn't work well: The
> diagnostic issued from a failed _Static_assert() would preferably
> refer to the original source expression, not an already macro
> expanded variant of it. That, however, precludes passing it
> through multiple levels of macro expansion. Which would leave
> passing e and #e to __ASSERT_STRUCT_FIELD(), but that's again
> ugly as it opens the door for someone adding a use where the two
> arguments don't match up.
Good point, but this is going to be pretty damn obscure. In fact, I
don't think it'll see more than the use here.
>> __COUNTER__:
>> Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
>> this (using __COUNTER__ or __LINE__). 4.3 and above.
>
> The fallback to __LINE__ is not necessarily correct in all cases,
> namely when deep macro expansions result in two instances used
> on the same source line, or a use in a header matches line number
> wise a second use in another header or the source file.
>
> I considered to option of a fallback (and hence an abstraction in
> compiler*.h) when doing this, but I didn't want to do something
> that would break in perhaps vary obscure ways.
I was thinking of my own code in moduleparam.h, but elfnote.h does the
same trick. In both cases, it'll simply fail compile if we fallback and
__LINE__ isn't unique.
So again, I don't think we're going to see many uses, and no existing
use will bite us.
>> __compiletime_error():
>> I blame Arjan for this. It disappears if not implemented, which
>> is just lazy. BUILD_BUG_ON() does this right, and he didn't fix
>> that at the time :(
>
> Again, the name of the macro made me not use it, as the obvious
> fallback is a link time error. The only thing I would be agreeable to
> is something like __buildtime_error().
Yes, it's awkward to use. Your own usage here is the only correct way
to do it, AFAICT:
> #define __build_bug_on_failed(n) __build_bug_on_##n##_failed
> #define _BUILD_BUG_ON(n, condition) \
> do {\
> extern void __compiletime_error(#condition) \
> __build_bug_on_failed(n)(void); \
> if (condition) __build_bug_on_failed(n)(); \
> } while(0)
> #define BUILD_BUG_ON(condition...) _BUILD_BUG_ON(__COUNTER__, ##condition)
The ... is overkill here: sure it's general, but we don't allow that
currently, nor in the other BUILD_BUG_ON() definitions.
So I think this becomes:
#define _BUILD_BUG_ON(undefname, condition)\
do { \
extern void __compiletime_error(#condition) undefname(void); \
if (condition) undefname();\
} while(0)
#define BUILD_BUG_ON(condition) \
_BUILD_BUG_ON(__UNIQUE_ID(__build_bug_fail), (condition))
>> I'd say we have three patches here, really:
>>
>> 1) Add __ASSERT_STRUCT_FIELD(e) to compiler.h
>> 2) Add __UNIQUE_ID().
>> 3) Use them (I can think of at least one other place for __UNIQUE_ID()).
>>
>> Jan, do you want the glory? :) If not, I'll respin.
>
> Depending on the answers to the above (i.e. how much of it
> actually would need re-doing and re-testing), it may take me
> some time to get to this, but beyond that I'm fine with trying
> to improve this to everyone's satisfaction.
Yeah, it is kind of a lot of work for a little bikeshed. But putting
compiler tests in
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
>>> On 06.11.12 at 02:51, Rusty Russell wrote:
> Andrew Morton writes:
>
>> On Fri, 02 Nov 2012 14:47:40 +
>> "Jan Beulich" wrote:
>>
>>> This makes the resulting diagnostics quite a bit more useful.
>>
>> So asserts Jan, but to confirm it I would need to download, configure,
>> build and install a different gcc version, which sounds rather a hassle.
>>
>> So, please show us an exmple of these diagnostics in the changelog.
>>
>>> --- 3.7-rc3/include/linux/bug.h
>>> +++ 3.7-rc3-static-assert/include/linux/bug.h
>>> @@ -27,8 +27,15 @@ struct pt_regs;
>>> result (of value 0 and type size_t), so the expression can be used
>>> e.g. in a structure initializer (or where-ever else comma expressions
>>> aren't permitted). */
>>> +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
>>
>> This sort of logic is normally performed via the
>> include/linux/compiler*.h system. And
>>
>> grep __GNUC include/linux/*.h
>>
>> indicates that we've been pretty successful. Can we do that here too?
>>
>> (eg: suppose the Intel compiler supports _Static_assert?)
>
> Yeah, there are a lot of goodies here:
>
> _Static_assert:
> We could define __ASSERT_STRUCT_FIELD(e) for this:
> #define BUILD_BUG_ON_ZERO(e) \
> sizeof(struct { __ASSERT_STRUCT_FIELD(e); })
I considered something like this too, but it wouldn't work well: The
diagnostic issued from a failed _Static_assert() would preferably
refer to the original source expression, not an already macro
expanded variant of it. That, however, precludes passing it
through multiple levels of macro expansion. Which would leave
passing e and #e to __ASSERT_STRUCT_FIELD(), but that's again
ugly as it opens the door for someone adding a use where the two
arguments don't match up.
> __COUNTER__:
> Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
> this (using __COUNTER__ or __LINE__). 4.3 and above.
The fallback to __LINE__ is not necessarily correct in all cases,
namely when deep macro expansions result in two instances used
on the same source line, or a use in a header matches line number
wise a second use in another header or the source file.
I considered to option of a fallback (and hence an abstraction in
compiler*.h) when doing this, but I didn't want to do something
that would break in perhaps vary obscure ways.
> __compiletime_error():
> I blame Arjan for this. It disappears if not implemented, which
> is just lazy. BUILD_BUG_ON() does this right, and he didn't fix
> that at the time :(
Again, the name of the macro made me not use it, as the obvious
fallback is a link time error. The only thing I would be agreeable to
is something like __buildtime_error().
> I'd say we have three patches here, really:
>
> 1) Add __ASSERT_STRUCT_FIELD(e) to compiler.h
> 2) Add __UNIQUE_ID().
> 3) Use them (I can think of at least one other place for __UNIQUE_ID()).
>
> Jan, do you want the glory? :) If not, I'll respin.
Depending on the answers to the above (i.e. how much of it
actually would need re-doing and re-testing), it may take me
some time to get to this, but beyond that I'm fine with trying
to improve this to everyone's satisfaction.
Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
On 06.11.12 at 02:51, Rusty Russell ru...@rustcorp.com.au wrote:
Andrew Morton a...@linux-foundation.org writes:
On Fri, 02 Nov 2012 14:47:40 +
Jan Beulich jbeul...@suse.com wrote:
This makes the resulting diagnostics quite a bit more useful.
So asserts Jan, but to confirm it I would need to download, configure,
build and install a different gcc version, which sounds rather a hassle.
So, please show us an exmple of these diagnostics in the changelog.
--- 3.7-rc3/include/linux/bug.h
+++ 3.7-rc3-static-assert/include/linux/bug.h
@@ -27,8 +27,15 @@ struct pt_regs;
result (of value 0 and type size_t), so the expression can be used
e.g. in a structure initializer (or where-ever else comma expressions
aren't permitted). */
+#if __GNUC__ 4 || (__GNUC__ == 4 __GNUC_MINOR__ = 6)
This sort of logic is normally performed via the
include/linux/compiler*.h system. And
grep __GNUC include/linux/*.h
indicates that we've been pretty successful. Can we do that here too?
(eg: suppose the Intel compiler supports _Static_assert?)
Yeah, there are a lot of goodies here:
_Static_assert:
We could define __ASSERT_STRUCT_FIELD(e) for this:
#define BUILD_BUG_ON_ZERO(e) \
sizeof(struct { __ASSERT_STRUCT_FIELD(e); })
I considered something like this too, but it wouldn't work well: The
diagnostic issued from a failed _Static_assert() would preferably
refer to the original source expression, not an already macro
expanded variant of it. That, however, precludes passing it
through multiple levels of macro expansion. Which would leave
passing e and #e to __ASSERT_STRUCT_FIELD(), but that's again
ugly as it opens the door for someone adding a use where the two
arguments don't match up.
__COUNTER__:
Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
this (using __COUNTER__ or __LINE__). 4.3 and above.
The fallback to __LINE__ is not necessarily correct in all cases,
namely when deep macro expansions result in two instances used
on the same source line, or a use in a header matches line number
wise a second use in another header or the source file.
I considered to option of a fallback (and hence an abstraction in
compiler*.h) when doing this, but I didn't want to do something
that would break in perhaps vary obscure ways.
__compiletime_error():
I blame Arjan for this. It disappears if not implemented, which
is just lazy. BUILD_BUG_ON() does this right, and he didn't fix
that at the time :(
Again, the name of the macro made me not use it, as the obvious
fallback is a link time error. The only thing I would be agreeable to
is something like __buildtime_error().
I'd say we have three patches here, really:
1) Add __ASSERT_STRUCT_FIELD(e) to compiler.h
2) Add __UNIQUE_ID().
3) Use them (I can think of at least one other place for __UNIQUE_ID()).
Jan, do you want the glory? :) If not, I'll respin.
Depending on the answers to the above (i.e. how much of it
actually would need re-doing and re-testing), it may take me
some time to get to this, but beyond that I'm fine with trying
to improve this to everyone's satisfaction.
Jan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
Jan Beulich jbeul...@suse.com writes:
On 06.11.12 at 02:51, Rusty Russell ru...@rustcorp.com.au wrote:
Andrew Morton a...@linux-foundation.org writes:
On Fri, 02 Nov 2012 14:47:40 +
Jan Beulich jbeul...@suse.com wrote:
This makes the resulting diagnostics quite a bit more useful.
So asserts Jan, but to confirm it I would need to download, configure,
build and install a different gcc version, which sounds rather a hassle.
So, please show us an exmple of these diagnostics in the changelog.
--- 3.7-rc3/include/linux/bug.h
+++ 3.7-rc3-static-assert/include/linux/bug.h
@@ -27,8 +27,15 @@ struct pt_regs;
result (of value 0 and type size_t), so the expression can be used
e.g. in a structure initializer (or where-ever else comma expressions
aren't permitted). */
+#if __GNUC__ 4 || (__GNUC__ == 4 __GNUC_MINOR__ = 6)
This sort of logic is normally performed via the
include/linux/compiler*.h system. And
grep __GNUC include/linux/*.h
indicates that we've been pretty successful. Can we do that here too?
(eg: suppose the Intel compiler supports _Static_assert?)
Yeah, there are a lot of goodies here:
_Static_assert:
We could define __ASSERT_STRUCT_FIELD(e) for this:
#define BUILD_BUG_ON_ZERO(e) \
sizeof(struct { __ASSERT_STRUCT_FIELD(e); })
I considered something like this too, but it wouldn't work well: The
diagnostic issued from a failed _Static_assert() would preferably
refer to the original source expression, not an already macro
expanded variant of it. That, however, precludes passing it
through multiple levels of macro expansion. Which would leave
passing e and #e to __ASSERT_STRUCT_FIELD(), but that's again
ugly as it opens the door for someone adding a use where the two
arguments don't match up.
Good point, but this is going to be pretty damn obscure. In fact, I
don't think it'll see more than the use here.
__COUNTER__:
Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
this (using __COUNTER__ or __LINE__). 4.3 and above.
The fallback to __LINE__ is not necessarily correct in all cases,
namely when deep macro expansions result in two instances used
on the same source line, or a use in a header matches line number
wise a second use in another header or the source file.
I considered to option of a fallback (and hence an abstraction in
compiler*.h) when doing this, but I didn't want to do something
that would break in perhaps vary obscure ways.
I was thinking of my own code in moduleparam.h, but elfnote.h does the
same trick. In both cases, it'll simply fail compile if we fallback and
__LINE__ isn't unique.
So again, I don't think we're going to see many uses, and no existing
use will bite us.
__compiletime_error():
I blame Arjan for this. It disappears if not implemented, which
is just lazy. BUILD_BUG_ON() does this right, and he didn't fix
that at the time :(
Again, the name of the macro made me not use it, as the obvious
fallback is a link time error. The only thing I would be agreeable to
is something like __buildtime_error().
Yes, it's awkward to use. Your own usage here is the only correct way
to do it, AFAICT:
#define __build_bug_on_failed(n) __build_bug_on_##n##_failed
#define _BUILD_BUG_ON(n, condition) \
do {\
extern void __compiletime_error(#condition) \
__build_bug_on_failed(n)(void); \
if (condition) __build_bug_on_failed(n)(); \
} while(0)
#define BUILD_BUG_ON(condition...) _BUILD_BUG_ON(__COUNTER__, ##condition)
The ... is overkill here: sure it's general, but we don't allow that
currently, nor in the other BUILD_BUG_ON() definitions.
So I think this becomes:
#define _BUILD_BUG_ON(undefname, condition)\
do { \
extern void __compiletime_error(#condition) undefname(void); \
if (condition) undefname();\
} while(0)
#define BUILD_BUG_ON(condition) \
_BUILD_BUG_ON(__UNIQUE_ID(__build_bug_fail), (condition))
I'd say we have three patches here, really:
1) Add __ASSERT_STRUCT_FIELD(e) to compiler.h
2) Add __UNIQUE_ID().
3) Use them (I can think of at least one other place for __UNIQUE_ID()).
Jan, do you want the glory? :) If not, I'll respin.
Depending on the answers to the above (i.e. how much of it
actually would need re-doing and re-testing), it may take me
some time to get to this, but beyond that I'm fine with trying
to improve this to everyone's satisfaction.
Yeah, it is kind of a lot of work for a little bikeshed. But putting
compiler tests in bug.h is pretty icky too.
Below is a __UNIQUE_ID() patch, which I'd like anyway to clean up
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
Andrew Morton writes:
> On Fri, 02 Nov 2012 14:47:40 +
> "Jan Beulich" wrote:
>
>> This makes the resulting diagnostics quite a bit more useful.
>
> So asserts Jan, but to confirm it I would need to download, configure,
> build and install a different gcc version, which sounds rather a hassle.
>
> So, please show us an exmple of these diagnostics in the changelog.
>
>> --- 3.7-rc3/include/linux/bug.h
>> +++ 3.7-rc3-static-assert/include/linux/bug.h
>> @@ -27,8 +27,15 @@ struct pt_regs;
>> result (of value 0 and type size_t), so the expression can be used
>> e.g. in a structure initializer (or where-ever else comma expressions
>> aren't permitted). */
>> +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
>
> This sort of logic is normally performed via the
> include/linux/compiler*.h system. And
>
> grep __GNUC include/linux/*.h
>
> indicates that we've been pretty successful. Can we do that here too?
>
> (eg: suppose the Intel compiler supports _Static_assert?)
Yeah, there are a lot of goodies here:
_Static_assert:
We could define __ASSERT_STRUCT_FIELD(e) for this:
#define BUILD_BUG_ON_ZERO(e) \
sizeof(struct { __ASSERT_STRUCT_FIELD(e); })
__COUNTER__:
Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
this (using __COUNTER__ or __LINE__). 4.3 and above.
__compiletime_error():
I blame Arjan for this. It disappears if not implemented, which
is just lazy. BUILD_BUG_ON() does this right, and he didn't fix
that at the time :(
I'd say we have three patches here, really:
1) Add __ASSERT_STRUCT_FIELD(e) to compiler.h
2) Add __UNIQUE_ID().
3) Use them (I can think of at least one other place for __UNIQUE_ID()).
Jan, do you want the glory? :) If not, I'll respin.
Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
On Fri, 02 Nov 2012 14:47:40 + "Jan Beulich" wrote: > This makes the resulting diagnostics quite a bit more useful. So asserts Jan, but to confirm it I would need to download, configure, build and install a different gcc version, which sounds rather a hassle. So, please show us an exmple of these diagnostics in the changelog. > --- 3.7-rc3/include/linux/bug.h > +++ 3.7-rc3-static-assert/include/linux/bug.h > @@ -27,8 +27,15 @@ struct pt_regs; > result (of value 0 and type size_t), so the expression can be used > e.g. in a structure initializer (or where-ever else comma expressions > aren't permitted). */ > +#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6) This sort of logic is normally performed via the include/linux/compiler*.h system. And grep __GNUC include/linux/*.h indicates that we've been pretty successful. Can we do that here too? (eg: suppose the Intel compiler supports _Static_assert?) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
>>> On 05.11.12 at 03:19, Rusty Russell wrote:
> Jan Beulich writes:
>> @@ -54,6 +61,15 @@ struct pt_regs;
>> */
>> #ifndef __OPTIMIZE__
>> #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
>> +#elif __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)
>> +#define __build_bug_on_failed(n) __build_bug_on_##n##_failed
>> +#define _BUILD_BUG_ON(n, condition) \
>> +do {\
>> +extern void __compiletime_error(#condition) \
>> +__build_bug_on_failed(n)(void); \
>> +if (condition) __build_bug_on_failed(n)(); \
>> +} while(0)
>> +#define BUILD_BUG_ON(condition...) _BUILD_BUG_ON(__COUNTER__, ##condition)
>> #else
>> extern int __build_bug_on_failed;
>> #define BUILD_BUG_ON(condition) \
>
> Why does this depend on gcc version? Looks like residue from an attempt
> to use _Static_assert here?
That was the original hope, yes, but didn't work because of
certain uses BUILD_BUG_ON() in inline functions. Hence the
next best solution was to use __compiletime_error(), which
still gives a better diagnostic than what we have so far, but is
- see include/linux/compiler-gcc4.h - available only with gcc 4.3
and newer (simply using the construct even when expanding to
nothing - in include/linux/compile.h - wouldn't have the intended
effect of producing a compiler diagnostic at all, it would get
deferred to link time in _all_ cases, even if the compiler can tell
it's a constant).
Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
On 05.11.12 at 03:19, Rusty Russell ru...@rustcorp.com.au wrote:
Jan Beulich jbeul...@suse.com writes:
@@ -54,6 +61,15 @@ struct pt_regs;
*/
#ifndef __OPTIMIZE__
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#elif __GNUC__ 4 || (__GNUC__ == 4 __GNUC_MINOR__ = 3)
+#define __build_bug_on_failed(n) __build_bug_on_##n##_failed
+#define _BUILD_BUG_ON(n, condition) \
+do {\
+extern void __compiletime_error(#condition) \
+__build_bug_on_failed(n)(void); \
+if (condition) __build_bug_on_failed(n)(); \
+} while(0)
+#define BUILD_BUG_ON(condition...) _BUILD_BUG_ON(__COUNTER__, ##condition)
#else
extern int __build_bug_on_failed;
#define BUILD_BUG_ON(condition) \
Why does this depend on gcc version? Looks like residue from an attempt
to use _Static_assert here?
That was the original hope, yes, but didn't work because of
certain uses BUILD_BUG_ON() in inline functions. Hence the
next best solution was to use __compiletime_error(), which
still gives a better diagnostic than what we have so far, but is
- see include/linux/compiler-gcc4.h - available only with gcc 4.3
and newer (simply using the construct even when expanding to
nothing - in include/linux/compile.h - wouldn't have the intended
effect of producing a compiler diagnostic at all, it would get
deferred to link time in _all_ cases, even if the compiler can tell
it's a constant).
Jan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
On Fri, 02 Nov 2012 14:47:40 + Jan Beulich jbeul...@suse.com wrote: This makes the resulting diagnostics quite a bit more useful. So asserts Jan, but to confirm it I would need to download, configure, build and install a different gcc version, which sounds rather a hassle. So, please show us an exmple of these diagnostics in the changelog. --- 3.7-rc3/include/linux/bug.h +++ 3.7-rc3-static-assert/include/linux/bug.h @@ -27,8 +27,15 @@ struct pt_regs; result (of value 0 and type size_t), so the expression can be used e.g. in a structure initializer (or where-ever else comma expressions aren't permitted). */ +#if __GNUC__ 4 || (__GNUC__ == 4 __GNUC_MINOR__ = 6) This sort of logic is normally performed via the include/linux/compiler*.h system. And grep __GNUC include/linux/*.h indicates that we've been pretty successful. Can we do that here too? (eg: suppose the Intel compiler supports _Static_assert?) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
Andrew Morton a...@linux-foundation.org writes:
On Fri, 02 Nov 2012 14:47:40 +
Jan Beulich jbeul...@suse.com wrote:
This makes the resulting diagnostics quite a bit more useful.
So asserts Jan, but to confirm it I would need to download, configure,
build and install a different gcc version, which sounds rather a hassle.
So, please show us an exmple of these diagnostics in the changelog.
--- 3.7-rc3/include/linux/bug.h
+++ 3.7-rc3-static-assert/include/linux/bug.h
@@ -27,8 +27,15 @@ struct pt_regs;
result (of value 0 and type size_t), so the expression can be used
e.g. in a structure initializer (or where-ever else comma expressions
aren't permitted). */
+#if __GNUC__ 4 || (__GNUC__ == 4 __GNUC_MINOR__ = 6)
This sort of logic is normally performed via the
include/linux/compiler*.h system. And
grep __GNUC include/linux/*.h
indicates that we've been pretty successful. Can we do that here too?
(eg: suppose the Intel compiler supports _Static_assert?)
Yeah, there are a lot of goodies here:
_Static_assert:
We could define __ASSERT_STRUCT_FIELD(e) for this:
#define BUILD_BUG_ON_ZERO(e) \
sizeof(struct { __ASSERT_STRUCT_FIELD(e); })
__COUNTER__:
Used to make a unique id. Let's define __UNIQUE_ID(prefix) for
this (using __COUNTER__ or __LINE__). 4.3 and above.
__compiletime_error():
I blame Arjan for this. It disappears if not implemented, which
is just lazy. BUILD_BUG_ON() does this right, and he didn't fix
that at the time :(
I'd say we have three patches here, really:
1) Add __ASSERT_STRUCT_FIELD(e) to compiler.h
2) Add __UNIQUE_ID().
3) Use them (I can think of at least one other place for __UNIQUE_ID()).
Jan, do you want the glory? :) If not, I'll respin.
Thanks,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
Jan Beulich writes:
> This makes the resulting diagnostics quite a bit more useful.
>
> Signed-off-by: Jan Beulich
Nice. I'm a bit disappointed we can't just treat _Static_assert() as
void, like:
#define BUILD_BUG_ON_ZERO(e) (_Static_assert(!(e), "!(" #e ")"), 0)
> @@ -54,6 +61,15 @@ struct pt_regs;
> */
> #ifndef __OPTIMIZE__
> #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> +#elif __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)
> +#define __build_bug_on_failed(n) __build_bug_on_##n##_failed
> +#define _BUILD_BUG_ON(n, condition) \
> + do {\
> + extern void __compiletime_error(#condition) \
> + __build_bug_on_failed(n)(void); \
> + if (condition) __build_bug_on_failed(n)(); \
> + } while(0)
> +#define BUILD_BUG_ON(condition...) _BUILD_BUG_ON(__COUNTER__, ##condition)
> #else
> extern int __build_bug_on_failed;
> #define BUILD_BUG_ON(condition) \
Why does this depend on gcc version? Looks like residue from an attempt
to use _Static_assert here?
Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
Jan Beulich jbeul...@suse.com writes:
This makes the resulting diagnostics quite a bit more useful.
Signed-off-by: Jan Beulich jbeul...@suse.com
Nice. I'm a bit disappointed we can't just treat _Static_assert() as
void, like:
#define BUILD_BUG_ON_ZERO(e) (_Static_assert(!(e), !( #e )), 0)
@@ -54,6 +61,15 @@ struct pt_regs;
*/
#ifndef __OPTIMIZE__
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#elif __GNUC__ 4 || (__GNUC__ == 4 __GNUC_MINOR__ = 3)
+#define __build_bug_on_failed(n) __build_bug_on_##n##_failed
+#define _BUILD_BUG_ON(n, condition) \
+ do {\
+ extern void __compiletime_error(#condition) \
+ __build_bug_on_failed(n)(void); \
+ if (condition) __build_bug_on_failed(n)(); \
+ } while(0)
+#define BUILD_BUG_ON(condition...) _BUILD_BUG_ON(__COUNTER__, ##condition)
#else
extern int __build_bug_on_failed;
#define BUILD_BUG_ON(condition) \
Why does this depend on gcc version? Looks like residue from an attempt
to use _Static_assert here?
Thanks,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
This makes the resulting diagnostics quite a bit more useful.
Signed-off-by: Jan Beulich
---
include/linux/bug.h | 16
1 file changed, 16 insertions(+)
--- 3.7-rc3/include/linux/bug.h
+++ 3.7-rc3-static-assert/include/linux/bug.h
@@ -27,8 +27,15 @@ struct pt_regs;
result (of value 0 and type size_t), so the expression can be used
e.g. in a structure initializer (or where-ever else comma expressions
aren't permitted). */
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
+#define BUILD_BUG_ON_ZERO(e) \
+ sizeof(struct { _Static_assert(!(e), "!(" #e ")"); })
+#define BUILD_BUG_ON_NULL(e) \
+ ((void *)sizeof(struct { _Static_assert(!(e), "!(" #e ")"); }))
+#else
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
+#endif
/*
* BUILD_BUG_ON_INVALID() permits the compiler to check the validity of the
@@ -54,6 +61,15 @@ struct pt_regs;
*/
#ifndef __OPTIMIZE__
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#elif __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 3)
+#define __build_bug_on_failed(n) __build_bug_on_##n##_failed
+#define _BUILD_BUG_ON(n, condition)\
+ do {\
+ extern void __compiletime_error(#condition) \
+ __build_bug_on_failed(n)(void); \
+ if (condition) __build_bug_on_failed(n)(); \
+ } while(0)
+#define BUILD_BUG_ON(condition...) _BUILD_BUG_ON(__COUNTER__, ##condition)
#else
extern int __build_bug_on_failed;
#define BUILD_BUG_ON(condition)\
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] utilize _Static_assert() for BUILD_BUG_ON() when the compiler supports it
This makes the resulting diagnostics quite a bit more useful.
Signed-off-by: Jan Beulich jbeul...@suse.com
---
include/linux/bug.h | 16
1 file changed, 16 insertions(+)
--- 3.7-rc3/include/linux/bug.h
+++ 3.7-rc3-static-assert/include/linux/bug.h
@@ -27,8 +27,15 @@ struct pt_regs;
result (of value 0 and type size_t), so the expression can be used
e.g. in a structure initializer (or where-ever else comma expressions
aren't permitted). */
+#if __GNUC__ 4 || (__GNUC__ == 4 __GNUC_MINOR__ = 6)
+#define BUILD_BUG_ON_ZERO(e) \
+ sizeof(struct { _Static_assert(!(e), !( #e )); })
+#define BUILD_BUG_ON_NULL(e) \
+ ((void *)sizeof(struct { _Static_assert(!(e), !( #e )); }))
+#else
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
#define BUILD_BUG_ON_NULL(e) ((void *)sizeof(struct { int:-!!(e); }))
+#endif
/*
* BUILD_BUG_ON_INVALID() permits the compiler to check the validity of the
@@ -54,6 +61,15 @@ struct pt_regs;
*/
#ifndef __OPTIMIZE__
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#elif __GNUC__ 4 || (__GNUC__ == 4 __GNUC_MINOR__ = 3)
+#define __build_bug_on_failed(n) __build_bug_on_##n##_failed
+#define _BUILD_BUG_ON(n, condition)\
+ do {\
+ extern void __compiletime_error(#condition) \
+ __build_bug_on_failed(n)(void); \
+ if (condition) __build_bug_on_failed(n)(); \
+ } while(0)
+#define BUILD_BUG_ON(condition...) _BUILD_BUG_ON(__COUNTER__, ##condition)
#else
extern int __build_bug_on_failed;
#define BUILD_BUG_ON(condition)\
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
