Bug 23352

Summary:

Turn on more compiler warnings in the Mac build

Product:

WebKit

Reporter:

Darin Adler <darin>

Component:

WebCore Misc.

Assignee:

Darin Adler <darin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

andersca, mrowe, simon.fraser

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.5

Bug Depends on:

23120

Bug Blocks:

Attachments:

Description	Flags
work in progress	none
step 1 -- covers the simplest cases	none
step 2 -- slightly more complex fixes	none
step 3 -- noreturn	none
step 4 -- last couple of warnings	none
last step -- turn the warnings on	mrowe: review+

Darin Adler

Reported 2009-01-15 11:39:59 PST

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

Attachments
work in progress (18.01 KB, patch) 2009-01-16 09:11 PST, Darin Adler	no flags	Details Formatted Diff Diff
step 1 -- covers the simplest cases (9.16 KB, patch) 2009-01-19 20:49 PST, Darin Adler	no flags	Details Formatted Diff Diff
step 2 -- slightly more complex fixes (8.13 KB, patch) 2009-01-21 00:05 PST, Darin Adler	no flags	Details Formatted Diff Diff
step 3 -- noreturn (3.52 KB, patch) 2009-01-24 13:49 PST, Darin Adler	no flags	Details Formatted Diff Diff
step 4 -- last couple of warnings (4.17 KB, patch) 2009-01-24 13:49 PST, Darin Adler	no flags	Details Formatted Diff Diff
last step -- turn the warnings on (4.07 KB, patch) 2009-01-25 13:45 PST, Darin Adler	mrowe: review+	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Darin Adler

Comment 1 2009-01-16 09:11:10 PST

Created attachment 26796 [details] work in progress

Darin Adler

Comment 2 2009-01-16 09:58:53 PST

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

Comment 3 2009-01-19 20:49:04 PST

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

Mark Rowe (bdash)

Comment 4 2009-01-19 21:18:29 PST

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 5 2009-01-19 21:20:33 PST

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

Darin Adler

Comment 6 2009-01-20 10:26:06 PST

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

Comment 7 2009-01-21 00:05:14 PST

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

Anders Carlsson

Comment 8 2009-01-23 18:10:57 PST

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

Darin Adler

Comment 9 2009-01-23 18:40:28 PST

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

Comment 10 2009-01-24 13:49:27 PST

Created attachment 26998 [details] step 3 -- noreturn

Darin Adler

Comment 11 2009-01-24 13:49:52 PST

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

Darin Adler

Comment 12 2009-01-25 12:25:56 PST

Comment on attachment 26998 [details] step 3 -- noreturn Landed as trac.webkit.org/changeset/40230 so clearing review flag.

Darin Adler

Comment 13 2009-01-25 12:28:26 PST

Comment on attachment 26999 [details] step 4 -- last couple of warnings Landed as trac.webkit.org/changeset/40231 so clearing review flag.

Darin Adler

Comment 14 2009-01-25 12:39:42 PST

(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

Comment 15 2009-01-25 12:55:35 PST

(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

Comment 16 2009-01-25 13:45:15 PST

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

Mark Rowe (bdash)

Comment 17 2009-01-25 18:55:05 PST

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

Comment 18 2009-01-25 18:58:40 PST

(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

Comment 19 2009-01-25 19:02:33 PST

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

Note You need to log in before you can comment on or make changes to this bug.