bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: indsize: New module


From: Bruno Haible
Subject: Re: indsize: New module
Date: Thu, 03 Dec 2020 23:02:17 +0100
User-agent: KMail/5.1.3 (Linux/4.4.0-193-generic; KDE/5.18.0; x86_64; ; )

Hi Paul,

> > /* The user can define UNSIGNED_INDSIZE_T, to get a different set of 
> > compiler
> >     warnings.  */
> 
> I'm leery of this complexity, as it would change the semantics of what the 
> type 
> means. The type is not just about compiler warnings; it's about preferring 
> signed integer arithmetic whenever possible to avoid unexpected results 
> during 
> comparisons and to catch integer overflow.

With the _current_ tools (sanitizers) and the _current_ C standard, the choice
of a signed type is a right one. But, beyond the readability (for humans), 
there's
also the benefits we can get from future compiler enhancements. Clang has just
introduced a special case of range types in clang 11; we could very well have
range types in C compilers in 2 years, and possibly in the standard in 10 years.

For this reason I see the choice of a signed type as an _implementation_ detail.
Conceptually, for a type whose values are 0..2^31-1, it does not matter whether
it is considered signed or not. Except for binary operations, like the 
comparisons
that you mention, for which the compiler warnings will help us, both now and
10 years from now.

The purpose of the UNSIGNED_INDSIZE_T is that we can occasionally verify that
no code makes assumptions about 'idx_t' being signed, since such assumptions
would not be future-proof.

> > typedef ptrdiff_t indsize_t;
> 
> The type name "indsize_t" is a bit long and this would detract from 
> readability 
> in programs that use a lot of indexes. I suggest "idx_t" instead.  dfa.c is 
> already using this type name and it works well there.

OK, I've changed s/indsize/idx/ everywhere.

> dfa.c also has IDX_MAX.

Good point. Added.

> I suppose IDX_WIDTH would also be useful.

Added as well.

> I'm not so 
> sure about PRIdIDX etc. as those macros make programs hard to read and I 
> don't 
> see much point to hiding the fact that the type is equivalent to ptrdiff_t. 
> (dfa.c just uses %td etc. for printf formats.)

Yes, %tu or %zu should work equally well (except for possibly some pointless
warnings on 64-bit native Windows).

I'm pushing the attached patch.

I don't know how far 'dfa.c' can make use of idx.h, without annoying Arnold
with too many dependencies when he pulls in 'dfa.c' into gawk.

Bruno

Attachment: 0001-idx-New-module.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]