On 12/6/20 3:32 AM, Bruno Haible wrote:
Paul Eggert wrote:
Those all look good to me.
OK, I pushed them.
When I looked into this a bit more when hacking on canonicalize-lgpl, I
realized I spoke too soon. First, there's some cruft left over from when
idx_t could be unsigned. More important, defining IDX_WIDTH to be
PTRDIFF_WIDTH - 1 is confusing as this makes idx_t different from all other
signed types in that IDX_WIDTH does not count the sign bit. I can see why it
doesn't count the sign bit (the sign bit is always zero) but I can also see
why it should (shift counts are limited by the *_WIDTH macros). If we're
going to have a macro that stands for PTRDIFF_WIDTH - 1 it'd probably be
better to use a different naming convention, to avoid that confusion.
However, nobody's using any such macro now and possibly we'll never need it
so in the meantime I merely substituted a comment for the #define in the
attached patch.
Hope this is OK; further comments welcome of course.
From 03d1588fca2bffe92b08bb4f4ba69273f4843431 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Wed, 16 Dec 2020 23:52:24 -0800
Subject: [PATCH] idx: simplify IDX_MAX, remove IDX_WIDTH
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* lib/idx.h (IDX_MAX): Simplify by removing obsolete reference
to UNSIGNED_IDX_T.
(IDX_WIDTH): Remove, since it’s not used and its value
arguably should be PTRDIFF_WIDTH anyway.
---
ChangeLog | 8 ++++++++
lib/idx.h | 33 ++++++++++++++-------------------
2 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index d50539adc..445e2c074 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2020-12-17 Paul Eggert <egg...@cs.ucla.edu>
+
+ idx: simplify IDX_MAX, remove IDX_WIDTH
+ * lib/idx.h (IDX_MAX): Simplify by removing obsolete reference
+ to UNSIGNED_IDX_T.
+ (IDX_WIDTH): Remove, since it’s not used and its value
+ arguably should be PTRDIFF_WIDTH anyway.
+
2020-12-16 Bruno Haible <br...@clisp.org>
posix_spawn_file_actions_addfchdir-tests: Rename test.
diff --git a/lib/idx.h b/lib/idx.h
index 0095467dc..ad7d31a2b 100644
--- a/lib/idx.h
+++ b/lib/idx.h
@@ -21,11 +21,12 @@
/* Get ptrdiff_t. */
#include <stddef.h>
-/* Get PTRDIFF_WIDTH. */
+/* Get PTRDIFF_MAX. */
#include <stdint.h>
/* The type 'idx_t' holds an (array) index or an (object) size.
- Its implementation is a signed integer type, capable of holding the values
+ Its implementation promotes to a signed integer type,
+ which can hold the values
0..2^63-1 (on 64-bit platforms) or
0..2^31-1 (on 32-bit platforms).
@@ -87,32 +88,26 @@
or to the C standard. Several programming languages (Ada, Haskell,
Common Lisp, Pascal) already have range types. Such range types may
help producing good code and good warnings. The type 'idx_t' could
- then be typedef'ed to a (signed!) range type. */
+ then be typedef'ed to a range type that is signed after promotion. */
-#if 0
-/* In the future, idx_t could be typedef'ed to a signed range type. */
-/* Note: The clang "extended integer types", supported in Clang 11 or newer
+/* In the future, idx_t could be typedef'ed to a signed range type.
+ The clang "extended integer types", supported in Clang 11 or newer
<https://clang.llvm.org/docs/LanguageExtensions.html#extended-integer-types>,
are a special case of range types. However, these types don't support binary
operators with plain integer types (e.g. expressions such as x > 1).
Therefore, they don't behave like signed types (and not like unsigned types
either). So, we cannot use them here. */
-typedef <some_range_type> idx_t;
-#else
-/* Use the signed type 'ptrdiff_t' by default. */
+
+/* Use the signed type 'ptrdiff_t'. */
/* Note: ISO C does not mandate that 'size_t' and 'ptrdiff_t' have the same
- size, but it is so an all platforms we have seen since 1990. */
+ size, but it is so on all platforms we have seen since 1990. */
typedef ptrdiff_t idx_t;
-#endif
/* IDX_MAX is the maximum value of an idx_t. */
-#if defined UNSIGNED_IDX_T
-# define IDX_MAX SIZE_MAX
-#else
-# define IDX_MAX PTRDIFF_MAX
-#endif
-
-/* IDX_WIDTH is the number of bits in an idx_t (31 or 63). */
-#define IDX_WIDTH (PTRDIFF_WIDTH - 1)
+#define IDX_MAX PTRDIFF_MAX
+
+/* So far no need has been found for an IDX_WIDTH macro.
+ Perhaps there should be another macro IDX_VALUE_BITS that does not
+ count the sign bit and is therefore one less than PTRDIFF_WIDTH. */
#endif /* _IDX_H */
--
2.27.0
