* 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).