GNU bug report logs - #68267
[PATCH] maint: add attributes to two functions without side effects

Previous Next

Package: coreutils;

Reported by: Samuel Tardieu <sam <at> rfc1149.net>

Date: Fri, 5 Jan 2024 16:45:01 UTC

Severity: normal

Tags: patch

Done: Pádraig Brady <P <at> draigBrady.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 68267 in the body.
You can then email your comments to 68267 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-coreutils <at> gnu.org:
bug#68267; Package coreutils. (Fri, 05 Jan 2024 16:45:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Samuel Tardieu <sam <at> rfc1149.net>:
New bug report received and forwarded. Copy sent to bug-coreutils <at> gnu.org. (Fri, 05 Jan 2024 16:45:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Samuel Tardieu <sam <at> rfc1149.net>
To: bug-coreutils <at> gnu.org
Cc: Samuel Tardieu <sam <at> rfc1149.net>
Subject: [PATCH] maint: add attributes to two functions without side effects
Date: Fri,  5 Jan 2024 17:44:06 +0100
* src/date.c (res_width): This function computes its result solely
from the value of its parameter and qualifies for the const attribute.
* src/tee.c (get_next_out): This function has no side effect and
qualifies for the pure attribute.

Those two functions were flagged by GCC 12.3.0.
---
Resent here, I sent it to the coreutils@ mailing-list while the hacking
doc says to send it here.

 src/date.c | 1 +
 src/tee.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/date.c b/src/date.c
index 39fc0715d..03bf012e2 100644
--- a/src/date.c
+++ b/src/date.c
@@ -294,6 +294,7 @@ Show the local time for 9AM next Friday on the west coast of the US\n\
 /* Yield the number of decimal digits needed to output a time with the
    nanosecond resolution RES, without losing information.  */
 
+ATTRIBUTE_CONST
 static int
 res_width (long int res)
 {
diff --git a/src/tee.c b/src/tee.c
index 07d525c95..eb074427c 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -185,6 +185,7 @@ main (int argc, char **argv)
 /* Return the index of the first non-null descriptor after idx,
    or -1 if all are null.  */
 
+ATTRIBUTE_PURE
 static int
 get_next_out (FILE **descriptors, int nfiles, int idx)
 {
-- 
2.42.0





Reply sent to Pádraig Brady <P <at> draigBrady.com>:
You have taken responsibility. (Sat, 06 Jan 2024 15:35:01 GMT) Full text and rfc822 format available.

Notification sent to Samuel Tardieu <sam <at> rfc1149.net>:
bug acknowledged by developer. (Sat, 06 Jan 2024 15:35:02 GMT) Full text and rfc822 format available.

Message #10 received at 68267-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Samuel Tardieu <sam <at> rfc1149.net>, 68267-done <at> debbugs.gnu.org
Subject: Re: bug#68267: [PATCH] maint: add attributes to two functions without
 side effects
Date: Sat, 6 Jan 2024 15:34:07 +0000
On 05/01/2024 16:44, Samuel Tardieu wrote:
> * src/date.c (res_width): This function computes its result solely
> from the value of its parameter and qualifies for the const attribute.
> * src/tee.c (get_next_out): This function has no side effect and
> qualifies for the pure attribute.
> 
> Those two functions were flagged by GCC 12.3.0.

I'm guessing GCC 12 needs help here as it can't determine the loops are finite.
(as per: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109914 )

Though I'm not seeing this suggestion with GCC 13.2.1,
so perhaps GCC 12 can determine the loops are finite?

I'll apply this since GCC 13 is less that a year old,
but in general we try to avoid littering code like this.

thanks,
Pádraig




Information forwarded to bug-coreutils <at> gnu.org:
bug#68267; Package coreutils. (Sat, 06 Jan 2024 16:35:01 GMT) Full text and rfc822 format available.

Message #13 received at 68267 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 68267 <at> debbugs.gnu.org, P <at> draigBrady.com, sam <at> rfc1149.net
Subject: Re: bug#68267: [PATCH] maint: add attributes to two functions without
 side effects
Date: Sat, 6 Jan 2024 08:34:22 -0800
On 2024-01-06 07:34, Pádraig Brady wrote:
> Though I'm not seeing this suggestion with GCC 13.2.1,
> so perhaps GCC 12 can determine the loops are finite?
> 
> I'll apply this since GCC 13 is less that a year old,
> but in general we try to avoid littering code like this.

I would not apply this, as it's a bug in GCC's warning code that has 
been fixed. There is no need to apply these attributes to static 
functions - as I understand it, with current GCC, either GCC figures out 
the attributes anyway (so no need for humans to do it) or it doesn't (so 
it doesn't know to warn). The attributes can help efficiency a bit with 
small extern functions, but with small static functions they're not 
worth the trouble.

More generally, we don't need to cater to older compilers simply to 
suppress false-positive warnings. We can simply tell users that they can 
either ignore the harmless warnings or upgrade to the current GCC.

Although there are some exceptions to this guideline (e.g., .h files 
that are intended to be used by other people) this doesn't seem to be 
one of them.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 04 Feb 2024 12:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 137 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.