Hi Andy,
On Thu, 15 Jun 2023 01:05:40 +0300
Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> On Wed, Jun 14, 2023 at 11:34 PM Herve Codina <herve.codina@xxxxxxxxxxx> wrote:
> > On Wed, 14 Jun 2023 14:51:43 +0300
> > Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> > > On Wed, Jun 14, 2023 at 12:42 PM Herve Codina <herve.codina@xxxxxxxxxxx> wrote:
> > > > On Wed, 14 Jun 2023 12:02:57 +0300
> > > > Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> > > > > On Wed, Jun 14, 2023 at 10:49 AM Herve Codina <herve.codina@xxxxxxxxxxx> wrote:
>
> ...
>
> > > > > > + typeof(__array[0] + 0) __element = __array[--__len]; \
> > > > >
> > > > > Do we need the ' + 0' part?
> > > >
> > > > Yes.
> > > >
> > > > __array can be an array of const items and it is legitimate to get the
> > > > minimum value from const items.
> > > >
> > > > typeof(__array[0]) keeps the const qualifier but we need to assign __element
> > > > in the loop.
> > > > One way to drop the const qualifier is to get the type from a rvalue computed
> > > > from __array[0]. This rvalue has to have the exact same type with only the const
> > > > dropped.
> > > > '__array[0] + 0' was a perfect canditate.
> > >
> > > Seems like this also deserves a comment. But if the series is accepted
> > > as is, it may be done as a follow up.
> > >
> >
> > Finally not so simple ...
> > I did some deeper tests and the macros need to be fixed.
> >
> > I hope this one (with comments added) is correct:
> > --- 8 ---
> > /*
> > * Do not check the array parameter using __must_be_array().
> > * In the following legit use-case where the "array" passed is a simple pointer,
> > * __must_be_array() will return a failure.
> > * --- 8< ---
> > * int *buff
> > * ...
> > * min = min_array(buff, nb_items);
> > * --- 8< ---
> > *
> > * The first typeof(&(array)[0]) is needed in order to support arrays of both
> > * 'int *buff' and 'int buf[N]' types.
> > *
> > * typeof(__array[0] + 0) used for __element is needed as the array can be an
> > * array of const items.
> > * In order to discard the const qualifier use an arithmetic operation (rvalue).
>
>
> > * This arithmetic operation discard the const but also can lead to an integer
>
> discards
>
> > * promotion. For instance, a const s8 __array[0] lead to an int __element due
>
> leads
>
> > * to the promotion.
> > * In this case, simple min() or max() operation fails (type mismatch).
> > * Use min_t() or max_t() (op_t parameter) enforcing the type in order to avoid
> > * the min() or max() failure.
>
> This part perhaps can be avoided. See below.
>
> > */
> > #define __minmax_array(op_t, array, len) ({ \
> > typeof(&(array)[0]) __array = (array); \
> > typeof(len) __len = (len); \
> > typeof(__array[0] + 0) __element = __array[--__len]; \
> > while (__len--) \
> > __element = op_t(typeof(__array[0]), __element, __array[__len]); \
>
> But can't we instead have typeof(+(array[0])) in the definition of __element?
> There are also other possible solutions: a) _Generic() with listed
> const types to move them to non-const, and b) __auto_type (which is
> supported by GCC 4.9 and clang, but not in the C11 standard).
typeof(+(array[0])) keeps the promotion.
__auto_type works with my gcc-12 but not with a gcc-5.5. Depending on the
compiler version, it discards or keeps the const qualifier. For this reason
I would prefer to not use it.
Did the job using _Generic().
This lead to:
--- 8< ---
/*
* Remove a const qualifier
* _Generic(foo, type-name: association, ..., default: association) performs a
* comparison against the foo type (not the qualified type).
* Do not use the const keyword in the type-name as it will not match the
* unqualified type of foo.
*/
#define __unconst_type_cases(type) \
unsigned type: (unsigned type)0, \
signed type: (signed type)0
#define __unconst_typeof(x) typeof( \
_Generic((x), \
char: (char)0, \
__unconst_type_cases(char), \
__unconst_type_cases(short), \
__unconst_type_cases(int), \
__unconst_type_cases(long), \
__unconst_type_cases(long long), \
default: (x)))
/*
* Do not check the array parameter using __must_be_array().
* In the following legit use-case where the "array" passed is a simple pointer,
* __must_be_array() will return a failure.
* --- 8< ---
* int *buff
* ...
* min = min_array(buff, nb_items);
* --- 8< ---
*
* The first typeof(&(array)[0]) is needed in order to support arrays of both
* 'int *buff' and 'int buf[N]' types.
*
* The array can be an array of const items.
* typeof() keeps the const qualifier. Use __unconst_typeof() in order to
* discard the const qualifier for the __element variable.
*/
#define __minmax_array(op, array, len) ({ \
typeof(&(array)[0]) __array = (array); \
typeof(len) __len = (len); \
__unconst_typeof(__array[0]) __element = __array[--__len]; \
while (__len--) \
__element = op(__element, __array[__len]); \
__element; })
/**
* min_array - return minimum of values present in an array
* @array: array
* @len: array length
*
* Note that @len must not be zero (empty array).
*/
#define min_array(array, len) __minmax_array(min, array, len)
/**
* max_array - return maximum of values present in an array
* @array: array
* @len: array length
*
* Note that @len must not be zero (empty array).
*/
#define max_array(array, len) __minmax_array(max, array, len)
--- 8< ---
Do you think it looks good ?
For, the KUnit tests, I agree, it would be nice to have something.
I need some more substantial work to implement and run the test in KUnit
and the first task will be learning the KUnit test system.
I will do that but out of this series.
Thanks for your feedback and pointers,
Hervé
