On 25.06.2024 21:07, Andrew Cooper wrote:
> --- a/xen/include/xen/macros.h
> +++ b/xen/include/xen/macros.h
> @@ -59,6 +59,8 @@
> #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
> #endif
>
> +#define BUILD_ERROR(msg) asm ( ".error \"" msg "\"" )
I think this wants a comment, and one even beyond what is said for
BUILD_BUG_ON(). This is primarily to make clear to people when to use
which, i.e. I consider it important to mention here that it is intended
for code which, in the normal case, we expect to be DCE-d. The nature
of the condition used is also a relevant factor, as in some cases
BUILD_BUG_ON() simply cannot be used because something that really is
compile time constant isn't an integer constant expression. With
something to this effect:
Reviewed-by: Jan Beulich <jbeul...@suse.com>
I have another question / suggestion, though.
> --- a/xen/include/xen/self-tests.h
> +++ b/xen/include/xen/self-tests.h
> @@ -22,9 +22,9 @@
> typeof(fn(val)) real = fn(val); \
> \
> if ( !__builtin_constant_p(real) ) \
> - asm ( ".error \"'" STR(fn(val)) "' not compile-time constant\""
> ); \
> + BUILD_ERROR("'" STR(fn(val)) "' not compile-time constant"); \
> else if ( real != res ) \
> - asm ( ".error \"Compile time check '" STR(fn(val) == res) "'
> failed\"" ); \
> + BUILD_ERROR("Compile time check '" STR(fn(val) == res) "'
> failed"); \
> } while ( 0 )
While right here leaving the condition outside of the macro is
perhaps more natural, considering a case where there's just an if()
I wonder whether we shouldn't also (only?) have BUILD_ERROR_ON(),
better paralleling BUILD_BUG_ON():
BUILD_ERROR_ON(!__builtin_constant_p(real),
"'" STR(fn(val)) "' not compile-time constant");
It then becomes questionable whether a string literal needs passing,
or whether instead the condition couldn't just be stringified while
passing to the asm():
BUILD_ERROR_ON(!__builtin_constant_p(real));
The thing could even "return" the predicate, permitting
if ( !BUILD_ERROR_ON(!__builtin_constant_p(real) )
BUILD_ERROR_ON(real != res);
I realize though that there may be issues from this with unused values
being diagnosed by compiler and/or Eclair, when the "return value" is
not of interest.
I'd be fine with the respective transformation to be left for 4.20,
though. Yet of course churn-wise it would be better to get into final
shape right away.
Jan
