bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
* compile warnings when including avltree-list and gcc-warnings is enabled
@ 2014-09-05 14:37 Dylan Cali
  2014-09-05 15:51 ` Paul Eggert
  0 siblings, 1 reply; 11+ messages in thread
From: Dylan Cali @ 2014-09-05 14:37 UTC (permalink / raw)
  To: bug-gnulib

Hello,

After adding a couple gnulib modules to coreutils I get several
warnings.  As a result, make fails since -Werror is enabled by
default.  If I configure with --disable-gcc-warnings then the warnings
go away.  The culprit seems to be avltree-list:

diff --git bootstrap.conf bootstrap.conf
index c0b5f02..a362376 100644
--- bootstrap.conf
+++ bootstrap.conf
@@ -34,6 +34,7 @@ gnulib_modules="
   argv-iter
   assert
   autobuild
+  avltree-list
   backupfile
   base64
   buffer-lcm
@@ -268,6 +269,7 @@ gnulib_modules="
   xgetcwd
   xgetgroups
   xgethostname
+  xlist
   xmemcoll
   xnanosleep
   xprintf


lib/gl_avltree_list.c:63:1: error: no previous declaration for
'gl_avltree_list_check_invariants' [-Werror=missing-declarations]
 gl_avltree_list_check_invariants (gl_list_t list)
 ^
In file included from lib/gl_avltree_list.c:37:0:
lib/gl_avltree_list.c: In function 'gl_tree_next_node'
lib/gl_anytree_list2.h:104:1: error: function might be candidate for
attribute 'pure' if it is known to return normally
[-Werror=suggest-attribute=pure]
 gl_tree_next_node (gl_list_t list, gl_list_node_t node)
 ^
lib/gl_avltree_list.c: In function 'gl_tree_previous_node'
lib/gl_anytree_list2.h:122:1: error: function might be candidate for
attribute 'pure' if it is known to return normally
[-Werror=suggest-attribute=pure]
 gl_tree_previous_node (gl_list_t list, gl_list_node_t node)
 ^
lib/gl_avltree_list.c: In function 'node_at'
lib/gl_anytree_list2.h:141:1: error: function might be candidate for
attribute 'pure' if it is known to return normally
[-Werror=suggest-attribute=pure]
 node_at (gl_list_node_t root, size_t position)
 ^
lib/gl_avltree_list.c: In function 'check_invariants'
lib/gl_avltree_list.c:41:1: error: function might be candidate for
attribute 'pure' if it is known to return normally
[-Werror=suggest-attribute=pure]
 check_invariants (gl_list_node_t node, gl_list_node_t parent)
 ^
In file included from lib/gl_avltree_list.c:37:0:
lib/gl_avltree_list.c: In function 'gl_tree_get_at'
lib/gl_anytree_list2.h:166:1: error: function might be candidate for
attribute 'pure' if it is known to return normally
[-Werror=suggest-attribute=pure]
 gl_tree_get_at (gl_list_t list, size_t position)
 ^
lib/gl_avltree_list.c: In function 'gl_avltree_list_check_invariants'
lib/gl_avltree_list.c:63:1: error: function might be candidate for
attribute 'pure' if it is known to return normally
[-Werror=suggest-attribute=pure]
 gl_avltree_list_check_invariants (gl_list_t list)
 ^

Let me know if you need any further information.

Thanks,
Dylan


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: compile warnings when including avltree-list and gcc-warnings is enabled
  2014-09-05 14:37 compile warnings when including avltree-list and gcc-warnings is enabled Dylan Cali
@ 2014-09-05 15:51 ` Paul Eggert
  2014-09-05 16:11   ` Dylan Cali
  2014-09-06  7:05   ` Dylan Cali
  0 siblings, 2 replies; 11+ messages in thread
From: Paul Eggert @ 2014-09-05 15:51 UTC (permalink / raw)
  To: Dylan Cali, bug-gnulib

Thanks, it looks like some declarations are missing in the corresponding 
.h file, or are missing attributes that they should have.  Is that 
something you could write a patch for?


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: compile warnings when including avltree-list and gcc-warnings is enabled
  2014-09-05 15:51 ` Paul Eggert
@ 2014-09-05 16:11   ` Dylan Cali
  2014-09-06  7:05   ` Dylan Cali
  1 sibling, 0 replies; 11+ messages in thread
From: Dylan Cali @ 2014-09-05 16:11 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

Sure, I'll take a look when I have a moment.

Thanks,
Dylan

On Fri, Sep 5, 2014 at 10:51 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> Thanks, it looks like some declarations are missing in the corresponding .h
> file, or are missing attributes that they should have.  Is that something
> you could write a patch for?


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: compile warnings when including avltree-list and gcc-warnings is enabled
  2014-09-05 15:51 ` Paul Eggert
  2014-09-05 16:11   ` Dylan Cali
@ 2014-09-06  7:05   ` Dylan Cali
  2014-09-07  0:30     ` Paul Eggert
  1 sibling, 1 reply; 11+ messages in thread
From: Dylan Cali @ 2014-09-06  7:05 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 850 bytes --]

On Fri, Sep 5, 2014 at 10:51 AM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> Thanks, it looks like some declarations are missing in the corresponding .h
> file, or are missing attributes that they should have.  Is that something
> you could write a patch for?

Hi Paul,

Attached is a patch for the attribute and declaration warnings.  After
fixing those the next warning that popped up is:

lib/gl_avltree_list.c: In function 'gl_avltree_list_check_invariants':
lib/gl_avltree_list.c:66:5: error: statement with no effect
[-Werror=unused-value]
     check_invariants (list->root, NULL);
     ^

I'm not sure how to proceed with this one.  It seems gcc is just
confused (the statement definitely has an effect as that function
aborts if the checks fail).  Would it be appropriate to add a pragma
to ignore the warning for that function?

Thanks,
Dylan

[-- Attachment #2: fix-attr-declaration-warnings.patch --]
[-- Type: text/x-patch, Size: 3797 bytes --]

From b47d780b903b4ea1fcc4421c605dc2c0978c4c3c Mon Sep 17 00:00:00 2001
From: Dylan Cali <calid1984@gmail.com>
Date: Sat, 6 Sep 2014 01:52:31 -0500
Subject: [PATCH] build: fix attribute and declaration warnings

* tests/test-avltree_list.c: Remove gl_avltree_list_check_invariants
declaration
* lib/gl_avltree_list.h: Add gl_avltree_list_check_invariants
declaration from test-avltree_list.c
* lib/gl_avltree_list.c: (check_invariants): Add pure attribute
(gl_avltree_list_check_invariants): Add const attribute
* lib/gl_anytree_list2.h: Add pure attribute to (gl_tree_node_value),
(gl_tree_next_node), (gl_tree_previous_node), (node_at),
(gl_tree_get_at)
---
 lib/gl_anytree_list2.h    | 10 +++++-----
 lib/gl_avltree_list.c     |  4 ++--
 lib/gl_avltree_list.h     |  1 +
 tests/test-avltree_list.c |  2 --
 4 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/lib/gl_anytree_list2.h b/lib/gl_anytree_list2.h
index 70e59a5..05fde15 100644
--- a/lib/gl_anytree_list2.h
+++ b/lib/gl_anytree_list2.h
@@ -59,7 +59,7 @@ gl_tree_size (gl_list_t list)
   return (list->root != NULL ? list->root->branch_size : 0);
 }
 
-static const void *
+static const void * _GL_ATTRIBUTE_PURE
 gl_tree_node_value (gl_list_t list, gl_list_node_t node)
 {
   return node->value;
@@ -100,7 +100,7 @@ gl_tree_node_nx_set_value (gl_list_t list, gl_list_node_t node, const void *elt)
   return 0;
 }
 
-static gl_list_node_t
+static gl_list_node_t _GL_ATTRIBUTE_PURE
 gl_tree_next_node (gl_list_t list, gl_list_node_t node)
 {
   if (node->right != NULL)
@@ -118,7 +118,7 @@ gl_tree_next_node (gl_list_t list, gl_list_node_t node)
   return node;
 }
 
-static gl_list_node_t
+static gl_list_node_t _GL_ATTRIBUTE_PURE
 gl_tree_previous_node (gl_list_t list, gl_list_node_t node)
 {
   if (node->left != NULL)
@@ -137,7 +137,7 @@ gl_tree_previous_node (gl_list_t list, gl_list_node_t node)
 }
 
 /* Return the node at the given position < gl_tree_size (list).  */
-static gl_list_node_t
+static gl_list_node_t _GL_ATTRIBUTE_PURE
 node_at (gl_list_node_t root, size_t position)
 {
   /* Here we know that root != NULL.  */
@@ -162,7 +162,7 @@ node_at (gl_list_node_t root, size_t position)
   return node;
 }
 
-static const void *
+static const void * _GL_ATTRIBUTE_PURE
 gl_tree_get_at (gl_list_t list, size_t position)
 {
   gl_list_node_t node = list->root;
diff --git a/lib/gl_avltree_list.c b/lib/gl_avltree_list.c
index 1afe5ca..31ed334 100644
--- a/lib/gl_avltree_list.c
+++ b/lib/gl_avltree_list.c
@@ -37,7 +37,7 @@
 #include "gl_anytree_list2.h"
 
 /* For debugging.  */
-static unsigned int
+static unsigned int _GL_ATTRIBUTE_PURE
 check_invariants (gl_list_node_t node, gl_list_node_t parent)
 {
   unsigned int left_height =
@@ -59,7 +59,7 @@ check_invariants (gl_list_node_t node, gl_list_node_t parent)
 
   return 1 + (left_height > right_height ? left_height : right_height);
 }
-void
+void _GL_ATTRIBUTE_CONST
 gl_avltree_list_check_invariants (gl_list_t list)
 {
   if (list->root != NULL)
diff --git a/lib/gl_avltree_list.h b/lib/gl_avltree_list.h
index 7f09ff3..e8300f1 100644
--- a/lib/gl_avltree_list.h
+++ b/lib/gl_avltree_list.h
@@ -24,6 +24,7 @@
 extern "C" {
 #endif
 
+extern void gl_avltree_list_check_invariants (gl_list_t list);
 extern const struct gl_list_implementation gl_avltree_list_implementation;
 #define GL_AVLTREE_LIST &gl_avltree_list_implementation
 
diff --git a/tests/test-avltree_list.c b/tests/test-avltree_list.c
index 1c0331a..c877c09 100644
--- a/tests/test-avltree_list.c
+++ b/tests/test-avltree_list.c
@@ -25,8 +25,6 @@
 #include "progname.h"
 #include "macros.h"
 
-extern void gl_avltree_list_check_invariants (gl_list_t list);
-
 static const char *objects[15] =
   {
     "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o"
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: compile warnings when including avltree-list and gcc-warnings is enabled
  2014-09-06  7:05   ` Dylan Cali
@ 2014-09-07  0:30     ` Paul Eggert
  2014-09-07  1:42       ` Dylan Cali
  0 siblings, 1 reply; 11+ messages in thread
From: Paul Eggert @ 2014-09-07  0:30 UTC (permalink / raw)
  To: Dylan Cali; +Cc: bug-gnulib

Dylan Cali wrote:
> --- a/lib/gl_avltree_list.h
> +++ b/lib/gl_avltree_list.h
> @@ -24,6 +24,7 @@
>   extern "C" {
>   #endif
>
> +extern void gl_avltree_list_check_invariants (gl_list_t list);

On second thought this doesn't look wise, as the function is not 
expected to be exported to ordinary clients, only to the test cases.  So 
let's leave the .h file alone, and put a declaration in the .c file instead.

I suppose a pragma is the way to silence the unwanted diagnostics, yes.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: compile warnings when including avltree-list and gcc-warnings is enabled
  2014-09-07  0:30     ` Paul Eggert
@ 2014-09-07  1:42       ` Dylan Cali
  2014-09-08 14:27         ` Eric Blake
  0 siblings, 1 reply; 11+ messages in thread
From: Dylan Cali @ 2014-09-07  1:42 UTC (permalink / raw)
  To: Paul Eggert; +Cc: bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

On Sat, Sep 6, 2014 at 7:30 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> On second thought this doesn't look wise, as the function is not expected to
> be exported to ordinary clients, only to the test cases.  So let's leave the
> .h file alone, and put a declaration in the .c file instead.
>
> I suppose a pragma is the way to silence the unwanted diagnostics, yes.

Updated patch attached.  I moved the declaration to the .c file and
added the warning suppression for just that function.  Everything
builds without issue now.

[-- Attachment #2: fix-gcc-warnings.patch --]
[-- Type: text/x-patch, Size: 3993 bytes --]

From d8f6449d69401a820c4b3f5d45cee8c87063618c Mon Sep 17 00:00:00 2001
From: Dylan Cali <calid1984@gmail.com>
Date: Sat, 6 Sep 2014 01:52:31 -0500
Subject: [PATCH] build: fix gcc warnings

* tests/test-avltree_list.c: Remove gl_avltree_list_check_invariants
declaration
* lib/gl_avltree_list.c: Add gl_avltree_list_check_invariants
declaration from test-avltree_list.c. Fixes gcc warning about missing
declaration.
* lib/gl_avltree_list.c: (check_invariants): Add pure attribute.
(gl_avltree_list_check_invariants): Add const attribute. Also use a gcc
pragma to ignore a false positive warning about check_invariants being a
statement with no effect.
* lib/gl_anytree_list2.h: Add pure attribute to (gl_tree_node_value),
(gl_tree_next_node), (gl_tree_previous_node), (node_at),
(gl_tree_get_at)
---
 lib/gl_anytree_list2.h    | 10 +++++-----
 lib/gl_avltree_list.c     | 14 ++++++++++++--
 tests/test-avltree_list.c |  2 --
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/lib/gl_anytree_list2.h b/lib/gl_anytree_list2.h
index 70e59a5..05fde15 100644
--- a/lib/gl_anytree_list2.h
+++ b/lib/gl_anytree_list2.h
@@ -59,7 +59,7 @@ gl_tree_size (gl_list_t list)
   return (list->root != NULL ? list->root->branch_size : 0);
 }
 
-static const void *
+static const void * _GL_ATTRIBUTE_PURE
 gl_tree_node_value (gl_list_t list, gl_list_node_t node)
 {
   return node->value;
@@ -100,7 +100,7 @@ gl_tree_node_nx_set_value (gl_list_t list, gl_list_node_t node, const void *elt)
   return 0;
 }
 
-static gl_list_node_t
+static gl_list_node_t _GL_ATTRIBUTE_PURE
 gl_tree_next_node (gl_list_t list, gl_list_node_t node)
 {
   if (node->right != NULL)
@@ -118,7 +118,7 @@ gl_tree_next_node (gl_list_t list, gl_list_node_t node)
   return node;
 }
 
-static gl_list_node_t
+static gl_list_node_t _GL_ATTRIBUTE_PURE
 gl_tree_previous_node (gl_list_t list, gl_list_node_t node)
 {
   if (node->left != NULL)
@@ -137,7 +137,7 @@ gl_tree_previous_node (gl_list_t list, gl_list_node_t node)
 }
 
 /* Return the node at the given position < gl_tree_size (list).  */
-static gl_list_node_t
+static gl_list_node_t _GL_ATTRIBUTE_PURE
 node_at (gl_list_node_t root, size_t position)
 {
   /* Here we know that root != NULL.  */
@@ -162,7 +162,7 @@ node_at (gl_list_node_t root, size_t position)
   return node;
 }
 
-static const void *
+static const void * _GL_ATTRIBUTE_PURE
 gl_tree_get_at (gl_list_t list, size_t position)
 {
   gl_list_node_t node = list->root;
diff --git a/lib/gl_avltree_list.c b/lib/gl_avltree_list.c
index 1afe5ca..9c408ac 100644
--- a/lib/gl_avltree_list.c
+++ b/lib/gl_avltree_list.c
@@ -36,8 +36,10 @@
 /* Generic binary tree code.  */
 #include "gl_anytree_list2.h"
 
+extern void gl_avltree_list_check_invariants (gl_list_t list);
+
 /* For debugging.  */
-static unsigned int
+static unsigned int _GL_ATTRIBUTE_PURE
 check_invariants (gl_list_node_t node, gl_list_node_t parent)
 {
   unsigned int left_height =
@@ -59,13 +61,21 @@ check_invariants (gl_list_node_t node, gl_list_node_t parent)
 
   return 1 + (left_height > right_height ? left_height : right_height);
 }
-void
+
+/* GCC warns that check_invariants has no effect, but it does. Ignore
+   the false positive. */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-value"
+
+void _GL_ATTRIBUTE_CONST
 gl_avltree_list_check_invariants (gl_list_t list)
 {
   if (list->root != NULL)
     check_invariants (list->root, NULL);
 }
 
+#pragma GCC diagnostic pop
+
 const struct gl_list_implementation gl_avltree_list_implementation =
   {
     gl_tree_nx_create_empty,
diff --git a/tests/test-avltree_list.c b/tests/test-avltree_list.c
index 1c0331a..c877c09 100644
--- a/tests/test-avltree_list.c
+++ b/tests/test-avltree_list.c
@@ -25,8 +25,6 @@
 #include "progname.h"
 #include "macros.h"
 
-extern void gl_avltree_list_check_invariants (gl_list_t list);
-
 static const char *objects[15] =
   {
     "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o"
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: compile warnings when including avltree-list and gcc-warnings is enabled
  2014-09-07  1:42       ` Dylan Cali
@ 2014-09-08 14:27         ` Eric Blake
  2014-09-08 15:04           ` Dylan Cali
  0 siblings, 1 reply; 11+ messages in thread
From: Eric Blake @ 2014-09-08 14:27 UTC (permalink / raw)
  To: Dylan Cali, Paul Eggert; +Cc: bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 1194 bytes --]

On 09/06/2014 07:42 PM, Dylan Cali wrote:
> On Sat, Sep 6, 2014 at 7:30 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
>> On second thought this doesn't look wise, as the function is not expected to
>> be exported to ordinary clients, only to the test cases.  So let's leave the
>> .h file alone, and put a declaration in the .c file instead.
>>
>> I suppose a pragma is the way to silence the unwanted diagnostics, yes.
> 
> Updated patch attached.  I moved the declaration to the .c file and
> added the warning suppression for just that function.  Everything
> builds without issue now.
> 

> 
> +
> +/* GCC warns that check_invariants has no effect, but it does. Ignore
> +   the false positive. */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wunused-value"

Does this work for all supported versions of gcc? Or do you need to make
it conditional on new enough gcc (it's okay if warnings have to be
disabled to compile with older gcc, but not okay if the way to disable
warnings for newer gcc causes compilation failure in older gcc).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: compile warnings when including avltree-list and gcc-warnings is enabled
  2014-09-08 14:27         ` Eric Blake
@ 2014-09-08 15:04           ` Dylan Cali
  2014-09-08 23:09             ` Dylan Cali
  0 siblings, 1 reply; 11+ messages in thread
From: Dylan Cali @ 2014-09-08 15:04 UTC (permalink / raw)
  To: Eric Blake; +Cc: Paul Eggert, bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

On Sep 8, 2014 9:27 AM, "Eric Blake" <eblake@redhat.com> wrote:
> Does this work for all supported versions of gcc? Or do you need to make
> it conditional on new enough gcc (it's okay if warnings have to be
> disabled to compile with older gcc, but not okay if the way to disable
> warnings for newer gcc causes compilation failure in older gcc).

Yep you're right, looking at some other code of mine I have a conditional
checking for gcc > 3.7 before doing the push/pop, so it looks like that is
the min version for this pragma.  So wrap the pragma in a conditional, and
add a conditional in the Makefile turning off warnings altogether for gcc
<= 3.7?

[-- Attachment #2: Type: text/html, Size: 798 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: compile warnings when including avltree-list and gcc-warnings is enabled
  2014-09-08 15:04           ` Dylan Cali
@ 2014-09-08 23:09             ` Dylan Cali
  2014-09-16 16:05               ` Pádraig Brady
  0 siblings, 1 reply; 11+ messages in thread
From: Dylan Cali @ 2014-09-08 23:09 UTC (permalink / raw)
  To: Eric Blake; +Cc: Paul Eggert, bug-gnulib

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

On Mon, Sep 8, 2014 at 10:04 AM, Dylan Cali <calid1984@gmail.com> wrote:
> On Sep 8, 2014 9:27 AM, "Eric Blake" <eblake@redhat.com> wrote:
>> Does this work for all supported versions of gcc? Or do you need to make
>> it conditional on new enough gcc (it's okay if warnings have to be
>> disabled to compile with older gcc, but not okay if the way to disable
>> warnings for newer gcc causes compilation failure in older gcc).
>
> Yep you're right, looking at some other code of mine I have a conditional
> checking for gcc > 3.7 before doing the push/pop, so it looks like that is
> the min version for this pragma.  So wrap the pragma in a conditional, and
> add a conditional in the Makefile turning off warnings altogether for gcc <=
> 3.7?

Ok cool, it looks like this is mostly already implemented.  First, I
was wrong about the version of gcc that supports these pragmas, it's
4.6 [1].  And gnulib already uses the diagnostic pragmas elsewhere, so
I mimicked the conditional format used in other files.  Finally, the
makefile conditional is already done in the coreutils configure.ac
[2].

I've attached a patch with the updates, let me know if you see any
further issues.

Thanks,
Dylan

[1] https://gcc.gnu.org/ml/gcc-help/2011-01/msg00113.html
[2] https://github.com/coreutils/coreutils/blob/master/configure.ac#L97

[-- Attachment #2: fix-gcc-warnings.patch --]
[-- Type: text/x-patch, Size: 4158 bytes --]

From 71f4f452e4768606c6f81300887e3e9c6ca74a93 Mon Sep 17 00:00:00 2001
From: Dylan Cali <calid1984@gmail.com>
Date: Sat, 6 Sep 2014 01:52:31 -0500
Subject: [PATCH] build: fix gcc warnings

* tests/test-avltree_list.c: Remove gl_avltree_list_check_invariants
declaration
* lib/gl_avltree_list.c: Add gl_avltree_list_check_invariants
declaration from test-avltree_list.c. Fixes gcc warning about missing
declaration.
* lib/gl_avltree_list.c: (check_invariants): Add pure attribute.
(gl_avltree_list_check_invariants): Add const attribute. Also use a gcc
pragma to ignore a false positive warning about check_invariants being a
statement with no effect (only for gcc >= 4.6).
* lib/gl_anytree_list2.h: Add pure attribute to (gl_tree_node_value),
(gl_tree_next_node), (gl_tree_previous_node), (node_at),
(gl_tree_get_at)
---
 lib/gl_anytree_list2.h    | 10 +++++-----
 lib/gl_avltree_list.c     | 18 ++++++++++++++++--
 tests/test-avltree_list.c |  2 --
 3 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/lib/gl_anytree_list2.h b/lib/gl_anytree_list2.h
index 70e59a5..05fde15 100644
--- a/lib/gl_anytree_list2.h
+++ b/lib/gl_anytree_list2.h
@@ -59,7 +59,7 @@ gl_tree_size (gl_list_t list)
   return (list->root != NULL ? list->root->branch_size : 0);
 }
 
-static const void *
+static const void * _GL_ATTRIBUTE_PURE
 gl_tree_node_value (gl_list_t list, gl_list_node_t node)
 {
   return node->value;
@@ -100,7 +100,7 @@ gl_tree_node_nx_set_value (gl_list_t list, gl_list_node_t node, const void *elt)
   return 0;
 }
 
-static gl_list_node_t
+static gl_list_node_t _GL_ATTRIBUTE_PURE
 gl_tree_next_node (gl_list_t list, gl_list_node_t node)
 {
   if (node->right != NULL)
@@ -118,7 +118,7 @@ gl_tree_next_node (gl_list_t list, gl_list_node_t node)
   return node;
 }
 
-static gl_list_node_t
+static gl_list_node_t _GL_ATTRIBUTE_PURE
 gl_tree_previous_node (gl_list_t list, gl_list_node_t node)
 {
   if (node->left != NULL)
@@ -137,7 +137,7 @@ gl_tree_previous_node (gl_list_t list, gl_list_node_t node)
 }
 
 /* Return the node at the given position < gl_tree_size (list).  */
-static gl_list_node_t
+static gl_list_node_t _GL_ATTRIBUTE_PURE
 node_at (gl_list_node_t root, size_t position)
 {
   /* Here we know that root != NULL.  */
@@ -162,7 +162,7 @@ node_at (gl_list_node_t root, size_t position)
   return node;
 }
 
-static const void *
+static const void * _GL_ATTRIBUTE_PURE
 gl_tree_get_at (gl_list_t list, size_t position)
 {
   gl_list_node_t node = list->root;
diff --git a/lib/gl_avltree_list.c b/lib/gl_avltree_list.c
index 1afe5ca..0466029 100644
--- a/lib/gl_avltree_list.c
+++ b/lib/gl_avltree_list.c
@@ -36,8 +36,10 @@
 /* Generic binary tree code.  */
 #include "gl_anytree_list2.h"
 
+extern void gl_avltree_list_check_invariants (gl_list_t list);
+
 /* For debugging.  */
-static unsigned int
+static unsigned int _GL_ATTRIBUTE_PURE
 check_invariants (gl_list_node_t node, gl_list_node_t parent)
 {
   unsigned int left_height =
@@ -59,13 +61,25 @@ check_invariants (gl_list_node_t node, gl_list_node_t parent)
 
   return 1 + (left_height > right_height ? left_height : right_height);
 }
-void
+
+/* GCC warns that check_invariants has no effect, but it does. Ignore
+   the false positive. */
+#if (__GNUC__ == 4 && 6 <= __GNUC_MINOR__) || 4 < __GNUC__
+# pragma GCC diagnostic push
+# pragma GCC diagnostic ignored "-Wunused-value"
+#endif
+
+void _GL_ATTRIBUTE_CONST
 gl_avltree_list_check_invariants (gl_list_t list)
 {
   if (list->root != NULL)
     check_invariants (list->root, NULL);
 }
 
+#if (__GNUC__ == 4 && 6 <= __GNUC_MINOR__) || 4 < __GNUC__
+# pragma GCC diagnostic pop
+#endif
+
 const struct gl_list_implementation gl_avltree_list_implementation =
   {
     gl_tree_nx_create_empty,
diff --git a/tests/test-avltree_list.c b/tests/test-avltree_list.c
index 1c0331a..c877c09 100644
--- a/tests/test-avltree_list.c
+++ b/tests/test-avltree_list.c
@@ -25,8 +25,6 @@
 #include "progname.h"
 #include "macros.h"
 
-extern void gl_avltree_list_check_invariants (gl_list_t list);
-
 static const char *objects[15] =
   {
     "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m", "n", "o"
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: compile warnings when including avltree-list and gcc-warnings is enabled
  2014-09-08 23:09             ` Dylan Cali
@ 2014-09-16 16:05               ` Pádraig Brady
  2019-09-29 14:39                 ` Bruno Haible
  0 siblings, 1 reply; 11+ messages in thread
From: Pádraig Brady @ 2014-09-16 16:05 UTC (permalink / raw)
  To: Dylan Cali; +Cc: bug-gnulib, Paul Eggert, Eric Blake

On 09/09/2014 12:09 AM, Dylan Cali wrote:
> On Mon, Sep 8, 2014 at 10:04 AM, Dylan Cali <calid1984@gmail.com> wrote:
>> On Sep 8, 2014 9:27 AM, "Eric Blake" <eblake@redhat.com> wrote:
>>> Does this work for all supported versions of gcc? Or do you need to make
>>> it conditional on new enough gcc (it's okay if warnings have to be
>>> disabled to compile with older gcc, but not okay if the way to disable
>>> warnings for newer gcc causes compilation failure in older gcc).
>>
>> Yep you're right, looking at some other code of mine I have a conditional
>> checking for gcc > 3.7 before doing the push/pop, so it looks like that is
>> the min version for this pragma.  So wrap the pragma in a conditional, and
>> add a conditional in the Makefile turning off warnings altogether for gcc <=
>> 3.7?
> 
> Ok cool, it looks like this is mostly already implemented.  First, I
> was wrong about the version of gcc that supports these pragmas, it's
> 4.6 [1].  And gnulib already uses the diagnostic pragmas elsewhere, so
> I mimicked the conditional format used in other files.  Finally, the
> makefile conditional is already done in the coreutils configure.ac
> [2].
> 
> I've attached a patch with the updates, let me know if you see any
> further issues.
> 
> Thanks,
> Dylan
> 
> [1] https://gcc.gnu.org/ml/gcc-help/2011-01/msg00113.html
> [2] https://github.com/coreutils/coreutils/blob/master/configure.ac#L97

I don't think we need to worry about the pragma as it was a valid warning in this case.
To tell the compiler we want to discard the result in this context
we can use ignore_value() or the more appropriate (void) cast in this case.

Also it's best to leave the prototype in the tests.

I've adjusted and pushed your patch accordingly at:
http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=5549ef8

thanks!
Pádraig.

p.s. It's marked trivial for licensing reasons.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: compile warnings when including avltree-list and gcc-warnings is enabled
  2014-09-16 16:05               ` Pádraig Brady
@ 2019-09-29 14:39                 ` Bruno Haible
  0 siblings, 0 replies; 11+ messages in thread
From: Bruno Haible @ 2019-09-29 14:39 UTC (permalink / raw)
  To: bug-gnulib; +Cc: Eric Blake, Paul Eggert, Dylan Cali

On 2014-09-16 Pádraig Brady wrote:
> I've adjusted and pushed your patch accordingly at:
> http://git.sv.gnu.org/gitweb/?p=gnulib.git;a=commitdiff;h=5549ef8

Now, on CentOS 8, with GCC 8.2.1 RedHat variant, I get this warning:

gl_avltree_list.c:67:1: warning: 'const' attribute on function returning 'void' [-Wattributes]

The warning is explained in [1]:
  "Note that a function that has pointer arguments and examines the data
   pointed to must not be declared const."

[1] https://gcc.gnu.org/onlinedocs/gcc-8.2.0/gcc/Common-Function-Attributes.html

If some GCC versions suggested to add 'const' on this function, this suggestion
was mistaken and should be silenced. In other words, don't use
-Werror=suggest-attribute=const with that version of GCC.


2019-09-29  Bruno Haible  <bruno@clisp.org>

	avltree-list: Fix compilation warning (introduced on 2014-09-16).
	* lib/gl_avltree_list.c (gl_avltree_list_check_invariants): Remove
	'const' attribute.

diff --git a/lib/gl_avltree_list.c b/lib/gl_avltree_list.c
index 655eeac..6d9a537 100644
--- a/lib/gl_avltree_list.c
+++ b/lib/gl_avltree_list.c
@@ -62,7 +62,7 @@ check_invariants (gl_list_node_t node, gl_list_node_t parent)
   return 1 + (left_height > right_height ? left_height : right_height);
 }
 
-void _GL_ATTRIBUTE_CONST
+void
 gl_avltree_list_check_invariants (gl_list_t list)
 {
   if (list->root != NULL)



^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-09-29 14:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-05 14:37 compile warnings when including avltree-list and gcc-warnings is enabled Dylan Cali
2014-09-05 15:51 ` Paul Eggert
2014-09-05 16:11   ` Dylan Cali
2014-09-06  7:05   ` Dylan Cali
2014-09-07  0:30     ` Paul Eggert
2014-09-07  1:42       ` Dylan Cali
2014-09-08 14:27         ` Eric Blake
2014-09-08 15:04           ` Dylan Cali
2014-09-08 23:09             ` Dylan Cali
2014-09-16 16:05               ` Pádraig Brady
2019-09-29 14:39                 ` Bruno Haible

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).