360829 - Removal of compiler warnings in Tamarin builds

Status: VERIFIED FIXED

(Tamarin Graveyard :: Build Config, defect)

Description

[Steven Johnson]

Tamarin coding guidelines require clean, warning-free compiles at the highest pragmatic warning levels available for a given compiler. The current Tamarin tree has various warnings disabled for historical purposes; this bug is intended to accumulate patches that will allow re-enablement of as many compiler warnings as is deemed practical.

Comment 1

[Steven Johnson]

Attachment: Enabling of various warnings in XCode, VS2003, VS2005, with code changes as needed.

Comment 2

[Werner Sharp]

Comment on attachment 245688 [details] [diff] [review]
Enabling of various warnings in XCode, VS2003, VS2005, with code changes as needed.

Should kIncrementalMarkDelayTicks  and kMarkSweepBurstTicks  be uintptr values instead of uint64?

Comment 3

[Steven Johnson]

checked in patch. leaving open for subsequent warning enablement that is likely to occur over the short-term future.

Comment 4

[Steven Johnson]

Attachment: Enable and clean up "hidden virtual functions" warning.

Enable and remove all "hidden virtual function" warnings. Definitely latent bugs here and involved many renamings of getProperty -> getAtomProperty, getStringProperty, etc. Passed checkin test on mac and windows.

Also renamed "Release Debugger" and "Debug Debugger" builds to convert space to underscore, to make cygwin work easier.

Comment 5

[Steven Johnson]

Committed patch to trunk.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Usually when checking in something to cvs, the comment includes also the name of the reviewer.

Comment 7

[Steven Johnson]

Oops! Sorry. r=[e-mail]

Comment 8

[Jeff Walden [:Waldo]]

(In reply to comment #7)

Actually, I think what was meant is that the *CVS* comment includes the reviewer, so instead of:

> Bug 360829: Removal of compiler warnings in Tamarin. Enabled (and fixed) many
> hidden-virtual-function warnings.

for the checkin comment, you would have used:

> Bug 360829: Removal of compiler warnings in Tamarin. Enabled (and fixed) many
> hidden-virtual-function warnings.  r=edwsmith

(Note the r= at the end.)  In general, every CVS commit message should include:

- bug #
- brief summary of the bug being fixed, usually a sentence or two
- patch author (with email if author isn't a common contributor of patches) if
  the person doing the commit didn't write the patch, e.g. "Patch by J. Arthur
  Random <[e-mail]>"
- reviewers (usually in the form "r=foopy")

You can also trawl through commit messages made by other people to see what the usual style is:

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=day&mindate=2006-11-21+14%3A47%3A01&maxdate=1970-12-31+16%3A00%3A01&cvsroot=%2Fcvsroot

Comment 9

[Steven Johnson]

Thanks for the reminder. In theory, I know the expected protocol, but in practice, I'm still adjusting myself to it... hopefully I will avoid such omissions in the future :-)

Comment 10

[Chris Peyer]

Transfer Bug from Core->Tamarin to Tamarin Product.

Comment 11

[Dan Schaffer]

marking as resolved since fix was submitted.