This makes the resulting diagnostics quite a bit more useful.
Signed-off-by: Jan Beulich <[email protected]>
---
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) \
Jan Beulich <[email protected]> writes:
> This makes the resulting diagnostics quite a bit more useful.
>
> Signed-off-by: Jan Beulich <[email protected]>
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.
>>> On 05.11.12 at 03:19, Rusty Russell <[email protected]> wrote:
> Jan Beulich <[email protected]> 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
On Fri, 02 Nov 2012 14:47:40 +0000
"Jan Beulich" <[email protected]> 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?)
Andrew Morton <[email protected]> writes:
> On Fri, 02 Nov 2012 14:47:40 +0000
> "Jan Beulich" <[email protected]> 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.
>>> On 06.11.12 at 02:51, Rusty Russell <[email protected]> wrote:
> Andrew Morton <[email protected]> writes:
>
>> On Fri, 02 Nov 2012 14:47:40 +0000
>> "Jan Beulich" <[email protected]> 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
Jan Beulich <[email protected]> writes:
>>>> On 06.11.12 at 02:51, Rusty Russell <[email protected]> wrote:
>> Andrew Morton <[email protected]> writes:
>>
>>> On Fri, 02 Nov 2012 14:47:40 +0000
>>> "Jan Beulich" <[email protected]> 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
moduleparam.h.
Cheers,
Rusty.
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 <[email protected]>
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, __PASTE(__, __COUNTER__))
+
#if __GNUC_MINOR__ >= 5
/*
* Mark a position in code as unreachable. This can be used to
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index f430e41..c55ae87 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -42,6 +42,10 @@ extern void __chk_io_ptr(const volatile void __iomem *);
# define __rcu
#endif
+/* Indirect macros required for expanded argument pasting, eg. __LINE__. */
+#define ___PASTE(a,b) a##b
+#define __PASTE(a,b) ___PASTE(a,b)
+
#ifdef __KERNEL__
#ifdef __GNUC__
@@ -164,6 +168,11 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
(typeof(ptr)) (__ptr + (off)); })
#endif
+/* Not-quite-unique ID. */
+#ifndef __UNIQUE_ID
+# define __UNIQUE_ID(prefix) __PASTE(__PASTE(prefix, __), __LINE__)
+#endif
+
#endif /* __KERNEL__ */
#endif /* __ASSEMBLY__ */
>>> On 07.11.12 at 02:03, Rusty Russell <[email protected]> wrote:
> Jan Beulich <[email protected]> writes:
>>>>> On 06.11.12 at 02:51, Rusty Russell <[email protected]> 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 <[email protected]>
Acked-by: Jan Beulich <[email protected]>
> 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, __PASTE(__, __COUNTER__))
> +
> #if __GNUC_MINOR__ >= 5
> /*
> * Mark a position in code as unreachable. This can be used to
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index f430e41..c55ae87 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -42,6 +42,10 @@ extern void __chk_io_ptr(const volatile void __iomem *);
> # define __rcu
> #endif
>
> +/* Indirect macros required for expanded argument pasting, eg. __LINE__. */
> +#define ___PASTE(a,b) a##b
> +#define __PASTE(a,b) ___PASTE(a,b)
> +
> #ifdef __KERNEL__
>
> #ifdef __GNUC__
> @@ -164,6 +168,11 @@ void ftrace_likely_update(struct ftrace_branch_data *f,
> int val, int expect);
> (typeof(ptr)) (__ptr + (off)); })
> #endif
>
> +/* Not-quite-unique ID. */
> +#ifndef __UNIQUE_ID
> +# define __UNIQUE_ID(prefix) __PASTE(__PASTE(prefix, __), __LINE__)
> +#endif
> +
> #endif /* __KERNEL__ */
>
> #endif /* __ASSEMBLY__ */
Jan Beulich <[email protected]> writes:
>>>> On 07.11.12 at 02:03, Rusty Russell <[email protected]> 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 <[email protected]>
>
> Acked-by: Jan Beulich <[email protected]>
Thanks,
Rusty.