Re: Stack module and -Wsign-compare

2022-01-06 Thread Marc Nieper-Wißkirchen
Thank you, Paul. My local tests also show that it behaves better now.

Am Mi., 5. Jan. 2022 um 20:54 Uhr schrieb Paul Eggert :
>
> On 1/5/22 10:00, Bruno Haible wrote:
> > Another possible fix would be to change
> >size_t size;
> > to
> >idx_t size;
> > in the struct.
>
> Yes, that fits better into our strategy of preferring signed to unsigned
> types for sizes. Plus, it avoids a cast that's too powerful in C. I
> installed the attached.



Re: Stack module and -Wsign-compare

2022-01-05 Thread Paul Eggert

On 1/5/22 10:00, Bruno Haible wrote:

Another possible fix would be to change
   size_t size;
to
   idx_t size;
in the struct.


Yes, that fits better into our strategy of preferring signed to unsigned 
types for sizes. Plus, it avoids a cast that's too powerful in C. I 
installed the attached.From 403968da56573bdbdcab79adabddcdc270e05cc9 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 5 Jan 2022 11:37:26 -0800
Subject: [PATCH 1/2] stack: pacify gcc -Wsign-compare

* lib/stack.h (_GL_STACK_TYPE): Use idx_t for size too.
Suggested by Bruno Haible in:
https://lists.gnu.org/r/bug-gnulib/2022-01/msg00035.html
---
 ChangeLog   | 7 +++
 lib/stack.h | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index ad29aed978..609720a451 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2022-01-05  Paul Eggert  
+
+	stack: pacify gcc -Wsign-compare
+	* lib/stack.h (_GL_STACK_TYPE): Use idx_t for size too.
+	Suggested by Bruno Haible in:
+	https://lists.gnu.org/r/bug-gnulib/2022-01/msg00035.html
+
 2022-01-05  Bruno Haible  
 
 	Fix last commit.
diff --git a/lib/stack.h b/lib/stack.h
index 26fdbad770..b4c35535a7 100644
--- a/lib/stack.h
+++ b/lib/stack.h
@@ -77,7 +77,7 @@
 typedef struct
 {
   GL_STACK_ELEMENT *base;
-  size_t size;
+  idx_t size;
   idx_t allocated;
 } _GL_STACK_TYPE;
 
-- 
2.32.0

From 9d3ec33a02dccaaa930f839752304d1b8d0b903e Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 5 Jan 2022 11:51:38 -0800
Subject: [PATCH 2/2] stack: also update stack_size to return idx_t

* lib/stack.h (_GL_STACK_PREFIX (size)): Return idx_t, not size_t.
---
 ChangeLog   | 1 +
 NEWS| 3 +++
 lib/stack.h | 6 +++---
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 609720a451..4829d58e83 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,6 +4,7 @@
 	* lib/stack.h (_GL_STACK_TYPE): Use idx_t for size too.
 	Suggested by Bruno Haible in:
 	https://lists.gnu.org/r/bug-gnulib/2022-01/msg00035.html
+	(_GL_STACK_PREFIX (size)): Return idx_t, not size_t.
 
 2022-01-05  Bruno Haible  
 
diff --git a/NEWS b/NEWS
index 2ce1c73014..c13a115ada 100644
--- a/NEWS
+++ b/NEWS
@@ -66,6 +66,9 @@ User visible incompatible changes
 
 DateModules Changes
 
+2022-01-05  stack   This module now uses idx_t instead of size_t
+for indexes and counts.
+
 2021-08-27  base32  These modules now use idx_t instead of size_t
 base64  for indexes and counts.
 
diff --git a/lib/stack.h b/lib/stack.h
index b4c35535a7..28b9d7a542 100644
--- a/lib/stack.h
+++ b/lib/stack.h
@@ -17,7 +17,7 @@
 /* Written by Marc Nieper-Wißkirchen , 2020.  */
 
 /* This header file does not have include-guards as it is meant to be
-   includeable multiple times as long as the necessary defines have
+   includable multiple times as long as the necessary defines have
been set up.
 
A stack is implemented with a homogenous array of elements in
@@ -36,7 +36,7 @@
  Popping:ELEMENT element = stack_pop (&stack);
  Discarding: stack_discard (&stack);
  Top-of-stack:   ELEMENT element = stack_top (&stack);
- Size:   size_t size = stack_size (&stack);
+ Size:   idx_t size = stack_size (&stack);
 
Here, ELEMENT is the type to which GL_STACK_ELEMENT was defined when
this file was included.
@@ -152,7 +152,7 @@ _GL_STACK_PREFIX (top) (const _GL_STACK_TYPE *stack)
 }
 
 /* Return the currently stored number of elements in the stack.  */
-GL_STACK_STORAGECLASS _GL_ATTRIBUTE_PURE size_t
+GL_STACK_STORAGECLASS _GL_ATTRIBUTE_PURE idx_t
 _GL_STACK_PREFIX (size) (const _GL_STACK_TYPE *stack)
 {
   return stack->size;
-- 
2.32.0



Re: Stack module and -Wsign-compare

2022-01-05 Thread Bruno Haible
Marc Nieper-Wißkirchen:
> If "stack.h" is included by client code that is supposed to be
> compiled with "-Wsign-compare", the compiler will complain about the
> comparison on line 121.

Indeed: although we don't care about -Wsign-compare warnings in Gnulib
code, consumers of .h files from Gnulib may well do.

> The simplest fix is to replace the comparison by:
> 
> stack->size == (size_t) stack->allocated

Another possible fix would be to change
  size_t size;
to
  idx_t size;
in the struct.

Bruno






Stack module and -Wsign-compare

2022-01-05 Thread Marc Nieper-Wißkirchen
Commit 3dc36216f168f4e752b648b19d85eab32a037827 by Paul Eggert
introduced a regression in the stack module.

If "stack.h" is included by client code that is supposed to be
compiled with "-Wsign-compare", the compiler will complain about the
comparison on line 121.

There wasn't a problem before commit
3dc36216f168f4e752b648b19d85eab32a037827 because in the previous
version both the "size" and the "allocated" field of a stack had the
same (unsigned) type.

As telling clients to compile their code without "-Wsign-compare"
isn't an option, I would like to fix it.

The simplest fix is to replace the comparison by:

stack->size == (size_t) stack->allocated

This shouldn't hide any overflows because stack->allocated is
non-negative and holds a number of allocated items and the size_t type
is by definition large enough to hold any number of allocated items.

Marc