Rewrite wxLogXXX() macros to avoid "ambiguous else" warnings.
Use a dummy for loop instead of an if statement to avoid all problems with the dangling else clauses: both the need for an artificially inversed "if" to make the code like if ( something ) wxLogError("..."); else something-else; to work as expected and to avoid warnings given by some versions of g++ and clang for the code above advising to add explicit braces. Closes #11829. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@74735 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775
This commit is contained in:
@@ -12,6 +12,7 @@
|
|||||||
#define _WX_LOG_H_
|
#define _WX_LOG_H_
|
||||||
|
|
||||||
#include "wx/defs.h"
|
#include "wx/defs.h"
|
||||||
|
#include "wx/cpp.h"
|
||||||
|
|
||||||
// ----------------------------------------------------------------------------
|
// ----------------------------------------------------------------------------
|
||||||
// types
|
// types
|
||||||
@@ -1329,29 +1330,27 @@ WXDLLIMPEXP_BASE const wxChar* wxSysErrorMsg(unsigned long nErrCode = 0);
|
|||||||
// following arguments are not even evaluated which is good as it avoids
|
// following arguments are not even evaluated which is good as it avoids
|
||||||
// unnecessary overhead)
|
// unnecessary overhead)
|
||||||
//
|
//
|
||||||
// Note: the strange if/else construct is needed to make the following code
|
// Note: the strange (because executing at most once) for() loop because we
|
||||||
|
// must arrange for wxDO_LOG() to be at the end of the macro and using a
|
||||||
|
// more natural "if (IsLevelEnabled()) wxDO_LOG()" would result in wrong
|
||||||
|
// behaviour for the following code ("else" would bind to the wrong "if"):
|
||||||
//
|
//
|
||||||
// if ( cond )
|
// if ( cond )
|
||||||
// wxLogError("!!!");
|
// wxLogError("!!!");
|
||||||
// else
|
// else
|
||||||
// ...
|
// ...
|
||||||
//
|
//
|
||||||
// work as expected, without it the second "else" would match the "if"
|
// See also #11829 for the problems with other simpler approaches,
|
||||||
// inside wxLogError(). Unfortunately code like
|
// notably the need for two macros due to buggy __LINE__ in MSVC.
|
||||||
//
|
#define wxDO_LOG_IF_ENABLED_HELPER(level, loopvar) \
|
||||||
// if ( cond )
|
for ( bool loopvar = false; \
|
||||||
// wxLogError("!!!");
|
!loopvar && wxLog::IsLevelEnabled(wxLOG_##level, wxLOG_COMPONENT); \
|
||||||
//
|
loopvar = true ) \
|
||||||
// now provokes "suggest explicit braces to avoid ambiguous 'else'"
|
|
||||||
// warnings from g++ 4.3 and later with -Wparentheses on but they can be
|
|
||||||
// easily fixed by adding curly braces around wxLogError() and at least
|
|
||||||
// the code still does do the right thing.
|
|
||||||
#define wxDO_LOG_IF_ENABLED(level) \
|
|
||||||
if ( !wxLog::IsLevelEnabled(wxLOG_##level, wxLOG_COMPONENT) ) \
|
|
||||||
{} \
|
|
||||||
else \
|
|
||||||
wxDO_LOG(level)
|
wxDO_LOG(level)
|
||||||
|
|
||||||
|
#define wxDO_LOG_IF_ENABLED(level) \
|
||||||
|
wxDO_LOG_IF_ENABLED_HELPER(level, wxMAKE_UNIQUE_NAME(wxlogcheck))
|
||||||
|
|
||||||
// wxLogFatalError() is special as it can't be disabled
|
// wxLogFatalError() is special as it can't be disabled
|
||||||
#define wxLogFatalError wxDO_LOG(FatalError)
|
#define wxLogFatalError wxDO_LOG(FatalError)
|
||||||
#define wxVLogFatalError(format, argptr) wxDO_LOGV(FatalError, format, argptr)
|
#define wxVLogFatalError(format, argptr) wxDO_LOGV(FatalError, format, argptr)
|
||||||
|
@@ -169,6 +169,7 @@ private:
|
|||||||
CPPUNIT_TEST( CompatLogger2 );
|
CPPUNIT_TEST( CompatLogger2 );
|
||||||
#endif // WXWIN_COMPATIBILITY_2_8
|
#endif // WXWIN_COMPATIBILITY_2_8
|
||||||
CPPUNIT_TEST( SysError );
|
CPPUNIT_TEST( SysError );
|
||||||
|
CPPUNIT_TEST( NoWarnings );
|
||||||
CPPUNIT_TEST_SUITE_END();
|
CPPUNIT_TEST_SUITE_END();
|
||||||
|
|
||||||
void Functions();
|
void Functions();
|
||||||
@@ -182,6 +183,7 @@ private:
|
|||||||
void CompatLogger2();
|
void CompatLogger2();
|
||||||
#endif // WXWIN_COMPATIBILITY_2_8
|
#endif // WXWIN_COMPATIBILITY_2_8
|
||||||
void SysError();
|
void SysError();
|
||||||
|
void NoWarnings();
|
||||||
|
|
||||||
TestLog *m_log;
|
TestLog *m_log;
|
||||||
wxLog *m_logOld;
|
wxLog *m_logOld;
|
||||||
@@ -362,3 +364,23 @@ void LogTestCase::SysError()
|
|||||||
#endif // __MINGW32__
|
#endif // __MINGW32__
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void LogTestCase::NoWarnings()
|
||||||
|
{
|
||||||
|
// Check that "else" branch is [not] taken as expected and that this code
|
||||||
|
// compiles without warnings (which used to not be the case).
|
||||||
|
|
||||||
|
bool b = wxFalse;
|
||||||
|
if ( b )
|
||||||
|
wxLogError("Not logged");
|
||||||
|
else
|
||||||
|
b = !b;
|
||||||
|
|
||||||
|
CPPUNIT_ASSERT( b );
|
||||||
|
|
||||||
|
if ( b )
|
||||||
|
wxLogError("If");
|
||||||
|
else
|
||||||
|
CPPUNIT_FAIL("Should not be taken");
|
||||||
|
|
||||||
|
CPPUNIT_ASSERT_EQUAL( "If", m_log->GetLog(wxLOG_Error) );
|
||||||
|
}
|
||||||
|
Reference in New Issue
Block a user