https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119170

--- Comment #10 from Alejandro Colomar <alx at kernel dot org> ---
(In reply to Kang-Che Sung from comment #9)
> Is there still room to make comments about the proposal?

Yes.  This will not be voted for inclusion in the standard until around
2025-09, and the proposal must be ready more than one month before that date. 
So, we have until around 2025-07 to clear any issues and give me time to write
the definitive proposal.

> ### `_Widthof(Type)`
> 
> The main reason (to me) for justifying the inclusion of `_Widthof` in the
> standard is to allow easy retrieval of bit width of **bit-precise integer
> types a.k.a. `_BitInt(N)`**, as the traditional expression `sizeof(Type) *
> CHAR_BIT` won't work for retrieving the width of them.

Correct.

> The `_Widthof`
> operator working with traditional integer types is a plus but not a
> necessity.

Here's an example of a use case of _Widthof with normal integers:

$ grepc -htfd csrand_uniform32 .
static uint32_t
csrand_uniform32(uint32_t n)
{
        uint32_t  bound, rem;
        uint64_t  r, mult;

        if (n == 0)
                return csrand32();

        bound = -n % n;  // analogous to `2^32 % n`, since `x % y == (x-y) % y`

        do {
                r = csrand32();
                mult = r * n;
                rem = mult;  // analogous to `mult % 2^32`
        } while (rem < bound);  // p = (2^32 % n) / 2^32;  W.C.: n=2^31+1,
p=0.5

        r = mult >> WIDTHOF(n);  // analogous to `mult / 2^32`

        return r;
}

Admittedly, I could hard-code a 32 there (just like it's hard-coded in the
function name, and local variables).  However, I think that's more brittle, and
if I paste this code elsewhere and make the function work for uint64_t, I'd
like to need minimal changes, so WIDTHOF() helps make it more robust.

> 
> I think @alejandro-colomar can add the `sizeof` on `_BitInt(N)` issue as a
> primary motive for introducing `_Widthof` in the standard. (This doesn't
> change the standard text, just amendment on the Motive section.)

It's already there in the paper:

        #define WIDTHOF(x)  (sizeof(x) * CHAR_BIT)
        #define SMAXOF(T)   ((T) (((((T) 1 << (WIDTHOF(T) - 2)) - 1) << 1) +
1))
        #define UMAXOF(T)   ((T) -1)
        #define IS_SIGNED(x)  ((typeof(x)) -1 < 1)
        #define MAXOF(T)    ((T) (IS_SIGNED(T) ? SMAXOF(T) : UMAXOF(T))
        #define MINOF(T)    ((T) ~type_max(T))

        As can be seen from my implementations, they are hard to write
        correctly, and review.  Also, now that we have bit-precise
        integers, one may want to get these to work with them, which
        would make it even harder to implement them.

I show the implementation with CHAR_BIT and that making it work for _BitInt
would be harder (it would involve popcount, and can't make it a constant
expression at the moment).

> There is one thing that's is useful to clarify (can add it in a  footnote of
> the standard text): Should `_Widthof(bool)` be 1? By literal interpretation
> of the semantics it sounds like so, but it is useful to note that explicitly.

Yes, _Bool must have a width of 1.

> ### `_Minof(Type)` and `_Maxof(Type)`
> 
> I'm personally skeptical of these two. And, if I have the decision power, I
> wish these two operators be made as votes separate from the `_Widthof`
> voting.

I'm on the other side: I could live without _Widthof (except for the use within
csrand_uniform32(), I only use it for implementing _Minof and _Maxof), but
certainly need _Minof and _Maxof often.

The following code can be found in shadow-utils
<https://github.com/shadow-maint/shadow>

$ grepc -tu ' type_m[ia][nx]' lib* src/
lib/atoi/getnum.h:inline int
get_gid(const char *restrict gidstr, gid_t *restrict gid)
{
        return a2i(gid_t, gid, gidstr, NULL, 10, type_min(gid_t),
type_max(gid_t));
}
lib/atoi/getnum.h:inline int
get_pid(const char *restrict pidstr, pid_t *restrict pid)
{
        return a2i(pid_t, pid, pidstr, NULL, 10, 1, type_max(pid_t));
}
lib/atoi/getnum.h:inline int
get_uid(const char *restrict uidstr, uid_t *restrict uid)
{
        return a2i(uid_t, uid, uidstr, NULL, 10, type_min(uid_t),
type_max(uid_t));
}
lib/limits.c:static int setrlimit_value (unsigned int resource,
                            const char *value,
                            unsigned int multiplier)
{
        rlim_t         l, limit;
        struct rlimit  rlim;

        /* The "-" is special, not belonging to a strange negative limit.
         * It is infinity, in a controlled way.
         */
        if ('-' == value[0]) {
                limit = RLIM_INFINITY;

        } else {
                if (a2i(rlim_t, &l, value, NULL, 10, 0, type_max(rlim_t)) == -1
                    && errno != ENOTSUP)
                {
                        return 0;  // FIXME: We could instead throw an error,
though.
                }

                if (__builtin_mul_overflow(l, multiplier, &limit)) {
                        /* FIXME: Again, silent error handling...
                         * Wouldn't screaming make more sense?
                         */
                        return 0;
                }
        }

        rlim.rlim_cur = limit;
        rlim.rlim_max = limit;
        if (setrlimit (resource, &rlim) != 0) {
                return LOGIN_ERROR_RLIMIT;
        }
        return 0;
}
src/usermod.c:static struct id_range
getid_range(const char *str)
{
        id_t             first, last;
        const char       *pos;
        struct id_range  result = {
                .first = type_max(id_t),
                .last = type_min(id_t)
        };

        static_assert(is_same_type(id_t, uid_t), "");
        static_assert(is_same_type(id_t, gid_t), "");

        first = type_min(id_t);
        last = type_max(id_t);

        if (a2i(id_t, &first, str, &pos, 10, first, last) == -1
            && errno != ENOTSUP)
        {
                return result;
        }

        if ('-' != *pos++)
                return result;

        if (a2i(id_t, &last, pos, NULL, 10, first, last) == -1)
                return result;

        result.first = first;
        result.last = last;
        return result;
}

> 
> With the inclusion of `_Widthof`, and the mandating of two's complement
> representation for signed integers since C23, the `_Minof` and `_Maxof`
> expressions would be **implementable in one way only**.

Not really true.  I implement them differently than you.

alx@devuan:/srv/alx/src/shadow/shadow/master$ grepc type_max .
./lib/typetraits.h:#define type_max(T)                                         
                 \
(                                                                             \
        (T) (is_signed(T) ? stype_max(T) : utype_max(T))                      \
)
alx@devuan:/srv/alx/src/shadow/shadow/master$ grepc stype_max .
./lib/typetraits.h:#define stype_max(T)                                        
                 \
(                                                                             \
        (T) (((((T) 1 << (_Wdithof(T) - 2)) - 1) << 1) + 1)                   
\
)
alx@devuan:/srv/alx/src/shadow/shadow/master$ grepc utype_max .
./lib/typetraits.h:#define utype_max(T)                                        
                 \
(                                                                             \
        (T) -1                                                                \
)

Which means I'd have to go and check why both your implementation and mine do
the same thing.

> And that they are
> "hard to write correctly and review" (quote from the Motive of the proposal)
> doesn't apply.
> 
> ```c
> #define IS_SIGNED(T) ((T) -1 < 1)
> #define MAXOF(T) ((T) ((((T) 1 << (_Widthof(T) - 1 - IS_SIGNED(T))) - 1) * 2
> + 1))
> #define MINOF(T) ((T) ~MAXOF(T)) /* Always works with two's complement
> representation */
> ```

I certainly would prefer to avoid reviewing that code.  Or writing it.  I don't
feel safe.

> Yes, there is only one way to implement these, as like the code above.

As I showed, I have a different implementation.  This statement is false.

> 
> I have multiple concerns with the `_Minof` and `_Maxof` proposal currently:
> * There are not technically necessary, and when programs need them, they are
> trivially implementable. (Standardizing them as keywords would only
> introduce noise to the language.)

I wouldn't call the above trivial.
The programmer who had to review my implementation certainly didn't feel safe:
<https://github.com/shadow-maint/shadow/pull/896#discussion_r1659894055>

> * The keywords proposed `_Minof` and `_Maxof` can be easily confused with
> `MIN()` and `MAX()` macros that programs often defined for retrieving the
> least or greatest value among two or more expressions. And I see no
> discussions regarding that potential user (I mean programmer, not compiler
> writer) confusion.

I don't think they can be accidentally misused one in place of the other,
because one has two arguments and the other has one.

> * Why not less confusing keywords such as `_Typemin` or `_Typemax`? No
> discussion of this either.

Because keyword operators named *of always act on types.  'type' would be
redundant with 'of'.  And not having 'of' would be inconsistent.  Since I don't
think they can be confused, due to the number of arguments, I'm okay with
_Minof/_Maxof.

> 
> That's all I think.

Thanks for the feedback!

Reply via email to