New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PATCH] PERL_UNUSED_RESULT: hopefully better way to ignore results (ditch V_Gconvert) #13781
Comments
From @jhiI like having less noise in my OS X build... the V_Gconvert isn't Blurb from comment: +/* Use PERL_UNUSED_RESULT() to suppress the warnings about unused results Patch attached. |
From @jhi0001-Add-PERL_UNUSED_RESULT-and-use-that-instead-of-the-V.patchFrom 062f092deef28f97906bfb6781a03786daecf5ab Mon Sep 17 00:00:00 2001
From: Jarkko Hietaniemi <jhi@iki.fi>
Date: Sun, 27 Apr 2014 20:52:54 -0400
Subject: [PATCH] Add PERL_UNUSED_RESULT() and use that instead of the
V_Gconvert().
---
perl.h | 26 ++++++++++++++++++++++++++
sv.c | 19 +++++--------------
2 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/perl.h b/perl.h
index 6da39f3..b740d9a 100644
--- a/perl.h
+++ b/perl.h
@@ -327,6 +327,32 @@
# define PERL_UNUSED_CONTEXT
#endif
+/* Use PERL_UNUSED_RESULT() to suppress the warnings about unused results
+ * of function calls, e.g. PERL_UNUSED_RESULT(foo(a, b)). Use it sparingly,
+ * though, since usually the warning is there for a good reason,
+ * e.g. for realloc(): the new pointer is not necessarily the old pointer.
+ *
+ * But sometimes you just want to ignore the return value, e.g. on
+ * codepaths soon ending up in abort, or in "best effort" attempts.
+ * Sometimes you can capture the return value and use PERL_UNUSED_VAR
+ * on that.
+ *
+ * The combination of gcc -Wunused-result (part of -Wall) and the gcc
+ * warn_unused_result attribute cannot be silenced with (void).
+ *
+ * The __typeof__() is unused instead of typeof() since typeof() is
+ * not available under stricter ANSI modes, and because of compilers
+ * masquerading as gcc (clang and icc), we want exactly the gcc
+ * extension __typeof__ and nothing else.
+ */
+#ifndef PERL_UNUSED_RESULT
+# ifdef __GNUC__
+# define PERL_UNUSED_RESULT(v) ({ __typeof__(v) z = (v); (void)sizeof(z); })
+# else
+# define PERL_UNUSED_RESULT(v) ((void)(v))
+# endif
+#endif
+
/* on gcc (and clang), specify that a warning should be temporarily
* ignored; e.g.
*
diff --git a/sv.c b/sv.c
index 087606b..a35b011 100644
--- a/sv.c
+++ b/sv.c
@@ -50,13 +50,6 @@
* has a mandatory return value, even though that value is just the same
* as the buf arg */
-#define V_Gconvert(x,n,t,b) \
-{ \
- char *rc = (char *)Gconvert(x,n,t,b); \
- PERL_UNUSED_VAR(rc); \
-}
-
-
#ifdef PERL_UTF8_CACHE_ASSERT
/* if adding more checks watch out for the following tests:
* t/op/index.t t/op/length.t t/op/pat.t t/op/substr.t
@@ -2954,12 +2947,12 @@ Perl_sv_2pv_flags(pTHX_ SV *const sv, STRLEN *const lp, const I32 flags)
/* some Xenix systems wipe out errno here */
#ifndef USE_LOCALE_NUMERIC
- V_Gconvert(SvNVX(sv), NV_DIG, 0, s);
+ PERL_UNUSED_RESULT(Gconvert(SvNVX(sv), NV_DIG, 0, s));
SvPOK_on(sv);
#else
{
DECLARE_STORE_LC_NUMERIC_SET_TO_NEEDED();
- V_Gconvert(SvNVX(sv), NV_DIG, 0, s);
+ PERL_UNUSED_RESULT(Gconvert(SvNVX(sv), NV_DIG, 0, s));
/* If the radix character is UTF-8, and actually is in the
* output, turn on the UTF-8 flag for the scalar */
@@ -10547,7 +10540,7 @@ Perl_sv_vcatpvfn_flags(pTHX_ SV *const sv, const char *const pat, const STRLEN p
if (digits && digits < sizeof(ebuf) - NV_DIG - 10) {
/* 0, point, slack */
STORE_LC_NUMERIC_SET_TO_NEEDED();
- V_Gconvert(nv, (int)digits, 0, ebuf);
+ PERL_UNUSED_RESULT(Gconvert(nv, (int)digits, 0, ebuf));
sv_catpv_nomg(sv, ebuf);
if (*ebuf) /* May return an empty string for digits==0 */
return;
@@ -11405,7 +11398,7 @@ Perl_sv_vcatpvfn_flags(pTHX_ SV *const sv, const char *const pat, const STRLEN p
aka precis is 0 */
if ( c == 'g' && precis) {
STORE_LC_NUMERIC_SET_TO_NEEDED();
- V_Gconvert((NV)nv, (int)precis, 0, PL_efloatbuf);
+ PERL_UNUSED_RESULT(Gconvert((NV)nv, (int)precis, 0, PL_efloatbuf));
/* May return an empty string for digits==0 */
if (*PL_efloatbuf) {
elen = strlen(PL_efloatbuf);
@@ -11836,7 +11829,6 @@ Perl_dirp_dup(pTHX_ DIR *const dp, CLONE_PARAMS *const param)
DIR *ret;
#if defined(HAS_FCHDIR) && defined(HAS_TELLDIR) && defined(HAS_SEEKDIR)
- int rc = 0;
DIR *pwd;
const Direntry_t *dirent;
char smallbuf[256];
@@ -11873,9 +11865,8 @@ Perl_dirp_dup(pTHX_ DIR *const dp, CLONE_PARAMS *const param)
/* Now we should have two dir handles pointing to the same dir. */
/* Be nice to the calling code and chdir back to where we were. */
- rc = fchdir(my_dirfd(pwd));
/* XXX If this fails, then what? */
- PERL_UNUSED_VAR(rc);
+ PERL_UNUSED_VAR(fchdir(my_dirfd(pwd)));
/* We have no need of the pwd handle any more. */
PerlDir_close(pwd);
--
1.9.2
|
From @tseeOn Tue Apr 29 05:33:40 2014, jhi wrote:
Reading the patch, there's two hunks at the end that look like this: @@ -11836,7 +11829,6 @@ Perl_dirp_dup(pTHX_ DIR *const dp, CLONE_PARAMS *const param) #if defined(HAS_FCHDIR) && defined(HAS_TELLDIR) && defined(HAS_SEEKDIR) /* Be nice to the calling code and chdir back to where we were. */ /* We have no need of the pwd handle any more. */ Did you mean to use your PERL_UNUSED_RESULT instead of PERL_UNUSED_VAR on this? I've applied the patch locally as is since I believe it's technically correct, but if the above was a brain fart, feel free to push a follow-up. |
The RT System itself - Status changed from 'new' to 'open' |
From @jhiOn Wednesday-201405-28, 4:32, Steffen Mueller via RT wrote:
Brainfart, yes. I meant _RESULT. |
From @tseeOn 05/28/2014 12:38 PM, Jarkko Hietaniemi wrote:
Will fix. --Steffen |
@tsee - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#121763 (status was 'resolved')
Searchable as RT121763$
The text was updated successfully, but these errors were encountered: