[ scummvm-Patches-2909981 ] create_msvc: Updated default warnings + per-project warnings

[SourceForge.net]

Patches item #2909981, was opened at 2009-12-07 07:28
Message generated for change (Tracker Item Submitted) made by littleboy
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=418822&aid=2909981&group_id=37116

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Julien (littleboy)
Assigned to: Nobody/Anonymous (nobody)
Summary: create_msvc: Updated default warnings + per-project warnings

Initial Comment:
There was some discussion a few days ago on the IRC channel about disabled warnings in Visual Studio. After removing all the default warnings and looking at VS throwing 10k+ warnings, I ended up with a list of warnings that can be disabled globally, a list of warnings that are never thrown and a list of warnings which might be useful.

This patch removes some of the default warnings and adds per-project warnings handling.

After the patch is applied, we are only left with 7 warnings (C4701: Potentially uninitialized local variable). Patches for most  of those are attached to the tracker entry (all those need review before applying!):
 - Gob: draw.cpp(869): initialize matchNum to 0 (protected by bestMatch anyway)
 - Gob: hotspots.cpp(1674): initialize deltax and deltay to 0 (not sure what was happening before)
 - Groovie: cell.cpp(775): initialize result to 0 (the only branch that doesn't sets a value has an assert)
 - Sci: gui_text.cpp(389): initialize offset to 0 and remove assignement in case SCI_TEXT_ALIGNMENT_LEFT. Not sure what the behavior was before in case of invalid aligment.
 - Sci: resource.cpp(995): initialize number to -1. processPatch already has a check for number == -1, so bAdd might be removed too (I'm not sure what resources values are possible, so needs close review by a sci engine dev)

 - Sci: state.cpp(281): no patch for this one as I have no idea what the default value for the Selector should be (and what side-effect it might have)

Kept the following warnings:
4068 (unknown pragma)
	only used in scumm engine to mark code sections
4100 (unreferenced formal parameter)
4103 (alignment changed after including header, may be due to missing #pragma pack(pop))
	used by pack-start / pack-end
4127 (conditional expression is constant)
	used in a lot of engines
4244 ('conversion' conversion from 'type1' to 'type2', possible loss of data)
	throws tons and tons of warnings, but would be nice to have it disabled on some projects if possible
4250 (inherits 'class2::member' via dominance)
4310 (cast truncates constant value)
	used in some engines, might be moved to per-engine later (there are too many of them right now)
4351 (new behavior: elements of array 'array' will be default initialized)
	a change in behavior in Visual Studio 2005. We want the new behavior, so can be disabled
4512 ('class' : assignment operator could not be generated)
4702 (unreachable code)
	mostly thrown after error() calls. Which of the supported platforms have compilers that do not support NORETURN?
4706 (assignment within conditional expression)
	used in a lot of engines
4800 ('type' : forcing value to bool 'true' or 'false' (performance warning))
	Not sure we really care about that one (and IIRC this is used in a lot of places)
4996 ('function': was declared deprecated)
	removes all the unsafe function warnings (strcpy_s, etc.)

Warnings that might be moved to per-engine (or resolved):
4121 (alignment of a member was sensitive to packing)
	Draci::GPL2Function, Draci::GPL2Operator, Kyra::ActiveWSA, Sci::SciSoundCommand are concerned.
4189 (local variable is initialized but not referenced)
	in lure only and the code looks fine, so maybe adding a pragma or disabling it only in that engine is ok
4355 ('this' : used in base member initializer list)
	kyra, lure, m4 and sci are affected
4510 ('class' : default constructor could not be generated)
	only agi is concerned
4610 (object 'class' can never be instantiated - user-defined constructor required)
	only agi is concerned

Removed the following warnings (never thrown):
4201 (nonstandard extension used : nameless struct/union)
4221 (nonstandard extension used : 'identifier' : cannot be initialized using address of automatic variable)
4267 ('var' : conversion from 'size_t' to 'type', possible loss of data)
4511 ('class' : copy constructor could not be generated)
4701 (Potentially uninitialized local variable 'name' used)


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=418822&aid=2909981&group_id=37116