From e03c3bb34fb7a2bdac847a23f10277285d9abf61 Mon Sep 17 00:00:00 2001 From: Ben Payne Date: Mon, 2 Feb 2015 18:17:37 -0500 Subject: [PATCH 1/3] Fix TORQUE_UNUSED for recent versions of MSVC Since there's now apparently no way to suppress the warning for a particular variable without adding at least some extra size to the executable, just turn the warning off in release builds. We leave it on in debug since it can sometimes help catch bugs, and we don't care about a little extra code in that configuration. --- Engine/source/platform/types.visualc.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Engine/source/platform/types.visualc.h b/Engine/source/platform/types.visualc.h index e383c1ea7..36d075d07 100644 --- a/Engine/source/platform/types.visualc.h +++ b/Engine/source/platform/types.visualc.h @@ -32,6 +32,16 @@ typedef signed _int64 S64; typedef unsigned _int64 U64; +// The types.h version of TORQUE_UNUSED no longer works for recent versions of MSVC. +// Since it appears that MS has made this impossible to do in a zero-overhead way, +// just turn the warning off in release builds. +#undef TORQUE_UNUSED +#ifdef TORQUE_DEBUG +#define TORQUE_UNUSED(var) ((0,0) ? (void)(var) : (void)0) +#else +#pragma warning(disable: 4189) // local variable is initialized but not referenced +#define TORQUE_UNUSED(var) ((void)0) +#endif //-------------------------------------- // Compiler Version From c19a70814c270d51dd768b0ffdfa45ec9cd42f39 Mon Sep 17 00:00:00 2001 From: Ben Payne Date: Mon, 2 Feb 2015 18:24:01 -0500 Subject: [PATCH 2/3] Tidy up and fix the various Assert macros Rephrase the macros so that they can be used in expressions, and properly require semicolons. And add the semicolons where missing. --- Engine/source/T3D/fx/fxFoliageReplicator.cpp | 6 +-- Engine/source/T3D/fx/fxShapeReplicator.cpp | 2 +- Engine/source/console/codeBlock.cpp | 2 +- Engine/source/core/idGenerator.h | 2 +- Engine/source/environment/timeOfDay.cpp | 4 +- .../source/gfx/bitmap/loaders/bitmapTga.cpp | 2 +- Engine/source/gui/core/guiCanvas.cpp | 3 +- Engine/source/platform/platformAssert.h | 53 +++++++++---------- Engine/source/sim/netGhost.cpp | 4 +- Engine/source/util/catmullRom.cpp | 2 +- Engine/source/util/catmullRom.h | 4 +- 11 files changed, 40 insertions(+), 44 deletions(-) diff --git a/Engine/source/T3D/fx/fxFoliageReplicator.cpp b/Engine/source/T3D/fx/fxFoliageReplicator.cpp index 7e88b174f..ec17c2f18 100644 --- a/Engine/source/T3D/fx/fxFoliageReplicator.cpp +++ b/Engine/source/T3D/fx/fxFoliageReplicator.cpp @@ -426,7 +426,7 @@ void fxFoliageReplicator::CreateFoliage(void) Point3F MaxPoint( 0.5, 0.5, 0.5 ); // Check Host. - AssertFatal(isClientObject(), "Trying to create Foliage on Server, this is bad!") + AssertFatal(isClientObject(), "Trying to create Foliage on Server, this is bad!"); // Cannot continue without Foliage Texture! if (dStrlen(mFieldData.mFoliageFile) == 0) @@ -1134,7 +1134,7 @@ void fxFoliageReplicator::ProcessQuadrant(fxFoliageQuadrantNode* pParentNode, fx void fxFoliageReplicator::SyncFoliageReplicators(void) { // Check Host. - AssertFatal(isServerObject(), "We *MUST* be on server when Synchronising Foliage!") + AssertFatal(isServerObject(), "We *MUST* be on server when Synchronising Foliage!"); // Find the Replicator Set. SimSet *fxFoliageSet = dynamic_cast(Sim::findObject("fxFoliageSet")); @@ -1196,7 +1196,7 @@ void fxFoliageReplicator::DestroyFoliageItems() void fxFoliageReplicator::DestroyFoliage(void) { // Check Host. - AssertFatal(isClientObject(), "Trying to destroy Foliage on Server, this is bad!") + AssertFatal(isClientObject(), "Trying to destroy Foliage on Server, this is bad!"); // Destroy Quad-tree. mPotentialFoliageNodes = 0; diff --git a/Engine/source/T3D/fx/fxShapeReplicator.cpp b/Engine/source/T3D/fx/fxShapeReplicator.cpp index 908212716..6093fa0d5 100644 --- a/Engine/source/T3D/fx/fxShapeReplicator.cpp +++ b/Engine/source/T3D/fx/fxShapeReplicator.cpp @@ -224,7 +224,7 @@ void fxShapeReplicator::CreateShapes(void) } // Check Shapes. - AssertFatal(mCurrentShapeCount==0,"Shapes already present, this should not be possible!") + AssertFatal(mCurrentShapeCount==0,"Shapes already present, this should not be possible!"); // Check that we have a shape... if (!mFieldData.mShapeFile) return; diff --git a/Engine/source/console/codeBlock.cpp b/Engine/source/console/codeBlock.cpp index 05a2ab798..9b87a5ed3 100644 --- a/Engine/source/console/codeBlock.cpp +++ b/Engine/source/console/codeBlock.cpp @@ -61,7 +61,7 @@ CodeBlock::CodeBlock() CodeBlock::~CodeBlock() { // Make sure we aren't lingering in the current code block... - AssertFatal(smCurrentCodeBlock != this, "CodeBlock::~CodeBlock - Caught lingering in smCurrentCodeBlock!") + AssertFatal(smCurrentCodeBlock != this, "CodeBlock::~CodeBlock - Caught lingering in smCurrentCodeBlock!"); if(name) removeFromCodeList(); diff --git a/Engine/source/core/idGenerator.h b/Engine/source/core/idGenerator.h index a0ddc74ed..8c3467673 100644 --- a/Engine/source/core/idGenerator.h +++ b/Engine/source/core/idGenerator.h @@ -74,7 +74,7 @@ public: void free(U32 id) { - AssertFatal(id >= mIdBlockBase, "IdGenerator::alloc: invalid id, id does not belong to this IdGenerator.") + AssertFatal(id >= mIdBlockBase, "IdGenerator::alloc: invalid id, id does not belong to this IdGenerator."); if(id == mNextId - 1) { mNextId--; diff --git a/Engine/source/environment/timeOfDay.cpp b/Engine/source/environment/timeOfDay.cpp index 49b069b64..a0ae2fc83 100644 --- a/Engine/source/environment/timeOfDay.cpp +++ b/Engine/source/environment/timeOfDay.cpp @@ -402,7 +402,7 @@ void TimeOfDay::_getSunColor( ColorF *outColor ) const //simple check if ( mColorTargets[0].elevation != 0.0f ) { - AssertFatal(0, "TimeOfDay::GetColor() - First elevation must be 0.0 radians") + AssertFatal(0, "TimeOfDay::GetColor() - First elevation must be 0.0 radians"); outColor->set(1.0f, 1.0f, 1.0f); //mBandMod = 1.0f; //mCurrentBandColor = color; @@ -411,7 +411,7 @@ void TimeOfDay::_getSunColor( ColorF *outColor ) const if ( mColorTargets[mColorTargets.size()-1].elevation != M_PI_F ) { - AssertFatal(0, "Celestails::GetColor() - Last elevation must be PI") + AssertFatal(0, "Celestails::GetColor() - Last elevation must be PI"); outColor->set(1.0f, 1.0f, 1.0f); //mBandMod = 1.0f; //mCurrentBandColor = color; diff --git a/Engine/source/gfx/bitmap/loaders/bitmapTga.cpp b/Engine/source/gfx/bitmap/loaders/bitmapTga.cpp index 15812ff9b..dff46bf03 100644 --- a/Engine/source/gfx/bitmap/loaders/bitmapTga.cpp +++ b/Engine/source/gfx/bitmap/loaders/bitmapTga.cpp @@ -483,7 +483,7 @@ static bool sReadTGA(Stream &stream, GBitmap *bitmap) static bool sWriteTGA(GBitmap *bitmap, Stream &stream, U32 compressionLevel) { - AssertISV(false, "GBitmap::writeTGA - doesn't support writing tga files!") + AssertISV(false, "GBitmap::writeTGA - doesn't support writing tga files!"); return false; } diff --git a/Engine/source/gui/core/guiCanvas.cpp b/Engine/source/gui/core/guiCanvas.cpp index 4dba87310..6c68e410c 100644 --- a/Engine/source/gui/core/guiCanvas.cpp +++ b/Engine/source/gui/core/guiCanvas.cpp @@ -712,7 +712,8 @@ bool GuiCanvas::processMouseEvent(InputEventInfo &inputEvent) // Need to query platform for specific things AssertISV(mPlatformWindow, "GuiCanvas::processMouseEvent - no window present!"); PlatformCursorController *pController = mPlatformWindow->getCursorController(); - AssertFatal(pController != NULL, "GuiCanvas::processInputEvent - No Platform Controller Found") + AssertFatal(pController != NULL, "GuiCanvas::processInputEvent - No Platform Controller Found"); + //copy the modifier into the new event mLastEvent.modifier = inputEvent.modifier; diff --git a/Engine/source/platform/platformAssert.h b/Engine/source/platform/platformAssert.h index c26f255c0..86ccb1b23 100644 --- a/Engine/source/platform/platformAssert.h +++ b/Engine/source/platform/platformAssert.h @@ -64,19 +64,17 @@ public: #ifdef TORQUE_ENABLE_ASSERTS - /*! - Assert that the statement x is true, and continue processing. +/*! + Assert that the statement x is true, and continue processing. - If the statment x is true, continue processing. + If the statment x is true, continue processing. - If the statement x is false, log the file and line where the assert occured, - the message y and continue processing. + If the statement x is false, log the file and line where the assert occured, + the message y and continue processing. - These asserts are only present in DEBUG builds. - */ - #define AssertWarn(x, y) \ - { if ((x)==0) \ - ::PlatformAssert::processAssert(::PlatformAssert::Warning, __FILE__, __LINE__, y); } + These asserts are only present in DEBUG builds. + */ +#define AssertWarn(x, y) (void)(!!(x) || ::PlatformAssert::processAssert(::PlatformAssert::Warning, __FILE__, __LINE__, y)) /*! Helper macro called when AssertFatal failed. @@ -86,27 +84,27 @@ public: #define ON_FAIL_ASSERTFATAL #endif - /*! - Assert that the statement x is true, otherwise halt. +/*! + Assert that the statement x is true, otherwise halt. - If the statement x is true, continue processing. + If the statement x is true, continue processing. - If the statement x is false, log the file and line where the assert occured, - the message y and displaying a dialog containing the message y. The user then - has the option to halt or continue causing the debugger to break. + If the statement x is false, log the file and line where the assert occured, + the message y and displaying a dialog containing the message y. The user then + has the option to halt or continue causing the debugger to break. - These asserts are only present in DEBUG builds. + These asserts are only present in DEBUG builds. - This assert is very useful for verifying data as well as function entry and - exit conditions. - */ - #define AssertFatal(x, y) \ - { if (((bool)(x))==false) \ - { if ( ::PlatformAssert::processAssert(::PlatformAssert::Fatal, __FILE__, __LINE__, y) ) { ::Platform::debugBreak(); } } } + This assert is very useful for verifying data as well as function entry and + exit conditions. + */ +#define AssertFatal(x, y) ((!(x) && ::PlatformAssert::processAssert(::PlatformAssert::Fatal, __FILE__, __LINE__, y)) ? ::Platform::debugBreak() : (void)0) \ #else - #define AssertFatal(x, y) { TORQUE_UNUSED(x); TORQUE_UNUSED(y); } - #define AssertWarn(x, y) { TORQUE_UNUSED(x); TORQUE_UNUSED(y); } + +#define AssertFatal(x, y) TORQUE_UNUSED(x) +#define AssertWarn(x, y) TORQUE_UNUSED(x) + #endif /*! @@ -121,10 +119,7 @@ public: This assert should only be used for rare conditions where the application cannot continue execution without seg-faulting and you want to display a nice exit message. */ -#define AssertISV(x, y) \ - { if ((x)==0) \ -{ if ( ::PlatformAssert::processAssert(::PlatformAssert::Fatal_ISV, __FILE__, __LINE__, y) ) { ::Platform::debugBreak(); } } } - +#define AssertISV(x, y) ((!(x) && ::PlatformAssert::processAssert(::PlatformAssert::Fatal_ISV, __FILE__, __LINE__, y)) ? ::Platform::debugBreak() : (void)0) \ /*! Sprintf style string formating into a fixed temporary buffer. diff --git a/Engine/source/sim/netGhost.cpp b/Engine/source/sim/netGhost.cpp index 13acfc7e3..9ea42fc06 100644 --- a/Engine/source/sim/netGhost.cpp +++ b/Engine/source/sim/netGhost.cpp @@ -958,7 +958,7 @@ void NetConnection::activateGhosting() // Iterate through the scope always objects... for (j = mGhostZeroUpdateIndex - 1; j >= 0; j--) { - AssertFatal((mGhostArray[j]->flags & GhostInfo::ScopeAlways) != 0, "NetConnection::activateGhosting: Non-scope always in the scope always list.") + AssertFatal((mGhostArray[j]->flags & GhostInfo::ScopeAlways) != 0, "NetConnection::activateGhosting: Non-scope always in the scope always list."); // Clear the ghost update mask and flags appropriately. mGhostArray[j]->updateMask = 0; @@ -1009,7 +1009,7 @@ void NetConnection::activateGhosting() // Iterate through the scope always objects... for (j = mGhostZeroUpdateIndex - 1; j >= 0; j--) { - AssertFatal((mGhostArray[j]->flags & GhostInfo::ScopeAlways) != 0, "NetConnection::activateGhosting: Non-scope always in the scope always list.") + AssertFatal((mGhostArray[j]->flags & GhostInfo::ScopeAlways) != 0, "NetConnection::activateGhosting: Non-scope always in the scope always list."); // Clear the ghost update mask and flags appropriately. mGhostArray[j]->updateMask = 0; diff --git a/Engine/source/util/catmullRom.cpp b/Engine/source/util/catmullRom.cpp index 10452c18c..7ac676a15 100644 --- a/Engine/source/util/catmullRom.cpp +++ b/Engine/source/util/catmullRom.cpp @@ -48,7 +48,7 @@ CatmullRomBase::CatmullRomBase() void CatmullRomBase::_initialize( U32 count, const F32 *times ) { //AssertFatal( times, "CatmullRomBase::_initialize() - Got null position!" ) - AssertFatal( count > 1, "CatmullRomBase::_initialize() - Must have more than 2 points!" ) + AssertFatal( count > 1, "CatmullRomBase::_initialize() - Must have more than 2 points!" ); // set up arrays mTimes = new F32[count]; diff --git a/Engine/source/util/catmullRom.h b/Engine/source/util/catmullRom.h index f4c9decb0..84bb272aa 100644 --- a/Engine/source/util/catmullRom.h +++ b/Engine/source/util/catmullRom.h @@ -142,8 +142,8 @@ inline void CatmullRom::clear() template inline void CatmullRom::initialize( U32 count, const TYPE *positions, const F32 *times ) { - AssertFatal( positions, "CatmullRom::initialize - Got null position!" ) - AssertFatal( count > 1, "CatmullRom::initialize - Must have more than 2 points!" ) + AssertFatal( positions, "CatmullRom::initialize - Got null position!" ); + AssertFatal( count > 1, "CatmullRom::initialize - Must have more than 2 points!" ); // Clean up any previous state. clear(); From 222be2bb723f6536bf33ce139e352dfc52c45e6f Mon Sep 17 00:00:00 2001 From: Ben Payne Date: Mon, 2 Feb 2015 19:11:20 -0500 Subject: [PATCH 3/3] Remove dead function --- Engine/source/platform/platformAssert.cpp | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/Engine/source/platform/platformAssert.cpp b/Engine/source/platform/platformAssert.cpp index 7e1d3b93a..f1a2b2957 100644 --- a/Engine/source/platform/platformAssert.cpp +++ b/Engine/source/platform/platformAssert.cpp @@ -69,24 +69,6 @@ bool PlatformAssert::displayMessageBox(const char *title, const char *message, b } static const char *typeName[] = { "Unknown", "Fatal-ISV", "Fatal", "Warning" }; -//------------------------------------------------------------------------------ -static bool askToEnterDebugger(const char* message ) -{ - static bool haveAsked = false; - static bool useDebugger = true; - if(!haveAsked ) - { - static char tempBuff[1024]; - dSprintf( tempBuff, 1024, "Torque has encountered an assertion with message\n\n" - "%s\n\n" - "Would you like to use the debugger? If you cancel, you won't be asked" - " again until you restart Torque.", message); - - useDebugger = Platform::AlertOKCancel("Use debugger?", tempBuff ); - haveAsked = true; - } - return useDebugger; -} //--------------------------------------