Bug 23352 – Turn on more compiler warnings in the Mac build

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Darin Adler]

I played around with some more compiler warnings we can turn on the Mac build. I'll attach my patch.

[Darin Adler]

Created attachment 26796 [details]
work in progress

[Darin Adler]

All I have to do is:

    1) Check that all these warnings and warning options work OK in the older version of the compiler we still use to build on Tiger.

    2) Check that none of these warning switches is redundant. Some of these warnings might already be on as a side effect of -Wall or -W or one of the Xcode warning options.

And then this could probably be landed any time. The elephant in the room, though, is -Woverloaded-virtual.

Also, we could use -Wextra instead of -W if we check that it's supported in all the versions of gcc we need to support.

[Darin Adler]

Created attachment 26854 [details]
step 1 -- covers the simplest cases

[Mark Rowe (bdash)]

One thing I noticed in the WIP patch is that you added -Wnewline-eof to WARNING_CFLAGS_BASE.  It'd be preferable to just set GCC_WARN_ABOUT_MISSING_NEWLINE = YES instead.

[Mark Rowe (bdash)]

Comment on attachment 26854 [details]
step 1 -- covers the simplest cases

r=me

[Darin Adler]

Comment on attachment 26854 [details]
step 1 -- covers the simplest cases

Landed this http://trac.webkit.org/changeset/40059 so clearing the review flag.

[Darin Adler]

Created attachment 26889 [details]
step 2 -- slightly more complex fixes

[Anders Carlsson]

Comment on attachment 26889 [details]
step 2 -- slightly more complex fixes

r=me

[Darin Adler]

Comment on attachment 26889 [details]
step 2 -- slightly more complex fixes

Clearing review flag because I landed this as <http://trac.webkit.org/changeset/40207>.

[Darin Adler]

Created attachment 26998 [details]
step 3 -- noreturn

[Darin Adler]

Created attachment 26999 [details]
step 4 -- last couple of warnings

[Darin Adler]

Comment on attachment 26998 [details]
step 3 -- noreturn

Landed as trac.webkit.org/changeset/40230 so clearing review flag.

[Darin Adler]

Comment on attachment 26999 [details]
step 4 -- last couple of warnings

Landed as trac.webkit.org/changeset/40231 so clearing review flag.

[Darin Adler]

(In reply to comment #4)
> One thing I noticed in the WIP patch is that you added -Wnewline-eof to
> WARNING_CFLAGS_BASE.  It'd be preferable to just set
> GCC_WARN_ABOUT_MISSING_NEWLINE = YES instead.

GCC_WARN_ABOUT_MISSING_NEWLINE = YES is already set, but for some reason -Wnewline-eof was *not* being passed in to the compiler.

[Darin Adler]

(In reply to comment #14)
> GCC_WARN_ABOUT_MISSING_NEWLINE = YES is already set, but for some reason
> -Wnewline-eof was *not* being passed in to the compiler.

My mistake. I think this was already set and already working.

[Darin Adler]

Created attachment 27026 [details]
last step -- turn the warnings on

[Mark Rowe (bdash)]

Comment on attachment 27026 [details]
last step -- turn the warnings on

r=me.  Are we going to turn these warnings on in WebKit as well?

[Darin Adler]

(In reply to comment #17)
> Are we going to turn these warnings on in WebKit as well?

Well, WebKit is not yet ready for uninitialized-parameter -- since it's Objective-C, there are a *lot* of those. But I did a diff on JavaScriptCore vs. WebCore and WebCore vs. WebKit to see if there were any interesting differences. I found a few that I want to talk to you about.

[Darin Adler]

http://trac.webkit.org/changeset/40237