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
Six mathoms need prototypes #15860
Comments
From @petdanceCreated by @petdanceThese six mathoms in mathoms.c need prototypes to satisfy clang's NV Perl_sv_2nv(pTHX_ SV *sv); char *Perl_sv_2pv(pTHX_ SV *sv, STRLEN *lp); NV Perl_huge(void); I32 Perl_sv_eq(pTHX_ SV *sv1, SV *sv2); char * Perl_sv_collxfrm(pTHX_ SV *const sv, STRLEN *const nxp); bool Perl_sv_2bool(pTHX_ SV *const sv); Perl Info
|
From @jkeenanOn Sat, 04 Feb 2017 22:02:00 GMT, petdance wrote:
Can you indicate how to configure and build Perl 5 blead so as to generate some of these warnings? On Linux x86_64, I built blead twice, once unthreaded, once threaded. ##### I then ran 'make test_prep' under 'script'. In each case, when I grepped for 'warning:', I came up with nothing. (I know we've cleaned up a lot of compiler warnings over the past year. When I tested perl-5.24.1 built with clang, I got a fair amount of warnings; see http://perl5.test-smoke.org/report/52884.) So I don't yet see what needs to be corrected. Thank you very much. -- |
The RT System itself - Status changed from 'new' to 'open' |
From @petdance
I'm working on getting us to run clean under clang -Weverything. These six mathoms are part of what needs to be cleaned up to get that to happen. Apply this patch and then configure with clang as your compiler. |
From @petdanceclang.patchdiff --git a/cflags.SH b/cflags.SH
index 3af1e97..7988b43 100755
--- a/cflags.SH
+++ b/cflags.SH
@@ -53,9 +53,33 @@ warn=''
case "$gccversion" in
'') ;;
Intel*) ;; # The Intel C++ plays gcc on TV but is not really it.
+# clang -Weverything is what gcc -Wall (+ -Wextra) was supposed to be.
+*" Clang "*|*"Apple LLVM "*)
+ case "$ccflags" in
+ *-Weverything*) ;;
+ *)
+ # -Wno-unused-macros: We have tons of macros and it's OK if we don't use them.
+ # -Wno-gnu-statement-expression: Commonly used all over the codebase.
+ # -Wno-shadow: We have many common global variable names. We would have to change many local variables.
+ # -Wno-overlength-strings: Compiler limit we don't care about
+ # -Wno-disabled-macro-expansion
+ # -Wno-variadic-macros: We use them.
+ # -Wno-cast-align
+ # -Wno-extended-offsetof
+ # -Wno-c11-extensions
+ # -Wno-format-nonliteral
+ # -Wno-switch-enum: This one is not mollified by a default in the case. https://stackoverflow.com/questions/16631713/
+ # -Wno-conversion: We have far too many conversions to clean up
+ # -Wno-missing-variable-declarations: globals.c barfs
+ echo "cflags.SH: Adding -Weverything -Wno-unused-macros -Wno-gnu-statement-expression -Wno-shadow -Wno-overlength-strings -Wno-padded -Wno-sign-conversion -Wno-shorten-64-to-32 -Wno-disabled-macro-expansion -Wno-variadic-macros -Wno-cast-align -Wno-extended-offsetof -Wno-c11-extensions -Wno-format-nonliteral -Wno-switch-enum -Wno-conversion -Wno-missing-variable-declarations"
+ warn="$warn -Weverything -Wno-unused-macros -Wno-gnu-statement-expression -Wno-shadow -Wno-overlength-strings -Wno-padded -Wno-sign-conversion -Wno-shorten-64-to-32 -Wno-disabled-macro-expansion -Wno-variadic-macros -Wno-cast-align -Wno-extended-offsetof -Wno-c11-extensions -Wno-format-nonliteral -Wno-switch-enum -Wno-conversion -Wno-missing-variable-declarations" ;;
+ esac
+ ;;
*) case "$ccflags" in
*-Wall*) ;;
- *) warn="$warn -Wall" ;;
+ *)
+ echo "cflags.SH: Adding -Wall."
+ warn="$warn -Wall" ;;
esac
;;
esac
@@ -247,6 +271,17 @@ Intel*) ;; # # Is that you, Intel C++?
;;
esac
;;
+ -Wextra)
+ # -Wextra is for (pure) gcc, not needed
+ # with clang -Weverything.
+ case " $warn " in
+ *-Weverything*) ;;
+ *)
+ echo "cflags.SH: Adding $opt."
+ warn="$warn $opt"
+ ;;
+ esac
+ ;;
*)
echo "cflags.SH: Adding $opt."
warn="$warn $opt"
|
From @jkeenanOn Sun, 05 Feb 2017 02:55:23 GMT, petdance wrote:
Yup, I see what you mean! Anyone else who wants to see a lot of warnings, checkout out this branch: smoke-me/jkeenan/petdance/130717-Weverything Thank you very much. -- |
From @petdanceI've done a lot of work on my branch to quiet those, but I'm not submitting at this time. Right now, all I'm doing is nothing that these six functions need prototypes. I may yet fix it myself. I'm marking it here for posterity. |
From @jkeenanOn Sun, 05 Feb 2017 03:24:14 GMT, jkeenan wrote:
A smoke-test of blead on freebsd-11 -- where clang is the default compiler -- with Andy's patch for -Weverything: http://perl5.test-smoke.org/report/53742 -- |
From @petdance
I didn't give you my entire patch. I just told how I did my configuration so that -Weverything is set up. I'm not sure why you set up a smoke test on this. |
From @demerphqOn 6 February 2017 at 03:09, Andy Lester via RT
Standard practice I should think. What you see might not be what everyone sees. Does it do any harm? I'm sure James was just trying to be helpful. Yves -- |
From @petdance
I wasn't criticizing. There was no subtext. I just don't understand why he set it up to smoke. |
From @demerphqOn 6 February 2017 at 05:28, Andy Lester via RT
I know more than once I've pushed a branch as a smoke-me when I only cheers, -- |
From @jkeenanOn Mon, 06 Feb 2017 02:09:56 GMT, petdance wrote:
I did that only so that if anyone else wanted to see the impact of clang + -Weverything on the number of warnings thrown by 'make', they could do so. At the present time we have next to no automated smoke testing of branches other than blead going on. (Dan Collins has two setups, neither of which use clang.) So other than the smoke tests which I manually kick off, your branch will hardly get smoke tested at all. I'll rename it by removing the smoke test part. Thank you very much. |
From @petdance
That's cool. It's just that that's only part of what I've done so far. I've reduced the amount of noise, but I didn't post those changes in this ticket. |
Migrated from rt.perl.org#130717 (status was 'open')
Searchable as RT130717$
The text was updated successfully, but these errors were encountered: