diff --git a/include/wx/cpp.h b/include/wx/cpp.h index c25c91fe1c..422ab032e9 100644 --- a/include/wx/cpp.h +++ b/include/wx/cpp.h @@ -96,6 +96,27 @@ #define wxSTATEMENT_MACRO_BEGIN do { #define wxSTATEMENT_MACRO_END } while ( (void)0, 0 ) +/* + Helper for executing the following statement conditionally without using + conditional statements. + + This strange macro is needed in the first place to avoid the problems due + to nested if/else inside macros. E.g. if some MACRO started with "if", then + + if ( cond ) + MACRO(); + else + ... + + would be broken because "..." would bind to the wrong "if" inside the macro + rather than the visible one. So we use wxDO_IF() inside the macro instead + to avoid this problem. + */ +#define wxDO_IF_HELPER(loopvar, condition) \ + for ( bool loopvar = false; !loopvar && condition; loopvar = true ) + +#define wxDO_IF(condition) wxDO_IF_HELPER(wxMAKE_UNIQUE_NAME(wxdoif), condition) + /* Define __WXFUNCTION__ which is like standard __FUNCTION__ but defined as NULL for the compilers which don't support the latter. diff --git a/include/wx/log.h b/include/wx/log.h index 514dc84b6b..90273b0457 100644 --- a/include/wx/log.h +++ b/include/wx/log.h @@ -1241,37 +1241,30 @@ WXDLLIMPEXP_BASE wxString wxSysErrorMsgStr(unsigned long nErrCode = 0); // this macro generates the expression which logs whatever follows it in // parentheses at the level specified as argument -#define wxDO_LOG(level) wxMAKE_LOGGER(level).Log +#define wxDO_LOG(level) wxDO_LOG_WITH_FUNC(level, Log) + +// generalization of the macro above that uses the given function of wxLogger +// object rather than the default "Log" +#define wxDO_LOG_WITH_FUNC(level, func) wxMAKE_LOGGER(level).func // this is the non-vararg equivalent #define wxDO_LOGV(level, format, argptr) \ wxMAKE_LOGGER(level).LogV(format, argptr) -// this macro declares wxLog() macro which logs whatever follows it if -// logging at specified level is enabled (notice that if it is false, the -// following arguments are not even evaluated which is good as it avoids -// unnecessary overhead) -// -// 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 ) -// wxLogError("!!!"); -// else -// ... -// -// See also #11829 for the problems with other simpler approaches, -// notably the need for two macros due to buggy __LINE__ in MSVC. -#define wxDO_LOG_IF_ENABLED_HELPER(level, loopvar) \ - for ( bool loopvar = false; \ - !loopvar && wxLog::IsLevelEnabled(wxLOG_##level, wxLOG_COMPONENT); \ - loopvar = true ) \ - wxDO_LOG(level) +// Macro evaluating to true if logging at the given level is enabled. +#define wxLOG_IS_ENABLED(level) \ + wxLog::IsLevelEnabled(wxLOG_##level, wxLOG_COMPONENT) +// Macro used to define most of the actual wxLogXXX() macros: just calls +// wxLogger::Log(), if logging at the specified level is enabled. #define wxDO_LOG_IF_ENABLED(level) \ - wxDO_LOG_IF_ENABLED_HELPER(level, wxMAKE_UNIQUE_NAME(wxlogcheck)) + wxDO_IF(wxLOG_IS_ENABLED(level)) \ + wxDO_LOG(level) + +// Similar to above, but calls the given function instead of Log(). +#define wxDO_LOG_IF_ENABLED_WITH_FUNC(level, func) \ + wxDO_IF(wxLOG_IS_ENABLED(level)) \ + wxDO_LOG_WITH_FUNC(level, func) // wxLogFatalError() is special as it can't be disabled #define wxLogFatalError wxDO_LOG(FatalError) @@ -1292,17 +1285,12 @@ WXDLLIMPEXP_BASE wxString wxSysErrorMsgStr(unsigned long nErrCode = 0); // this one is special as it only logs if we're in verbose mode #define wxLogVerbose \ - if ( !(wxLog::IsLevelEnabled(wxLOG_Info, wxLOG_COMPONENT) && \ - wxLog::GetVerbose()) ) \ - {} \ - else \ - wxDO_LOG(Info) + wxDO_IF(wxLOG_IS_ENABLED(Info) && wxLog::GetVerbose()) \ + wxDO_LOG(Info) + #define wxVLogVerbose(format, argptr) \ - if ( !(wxLog::IsLevelEnabled(wxLOG_Info, wxLOG_COMPONENT) && \ - wxLog::GetVerbose()) ) \ - {} \ - else \ - wxDO_LOGV(Info, format, argptr) + wxDO_IF(wxLOG_IS_ENABLED(Info) && wxLog::GetVerbose()) \ + wxDO_LOGV(Info, format, argptr) // another special case: the level is passed as first argument of the function // and so is not available to the macro @@ -1311,10 +1299,8 @@ WXDLLIMPEXP_BASE wxString wxSysErrorMsgStr(unsigned long nErrCode = 0); // always evaluated, unlike for the other log functions #define wxLogGeneric wxMAKE_LOGGER(Max).LogAtLevel #define wxVLogGeneric(level, format, argptr) \ - if ( !wxLog::IsLevelEnabled(wxLOG_##level, wxLOG_COMPONENT) ) \ - {} \ - else \ - wxDO_LOGV(level, format, argptr) + wxDO_IF(wxLOG_IS_ENABLED(level)) \ + wxDO_LOGV(level, format, argptr) // wxLogSysError() needs to stash the error code value in the log record info @@ -1331,12 +1317,9 @@ WXDLLIMPEXP_BASE wxString wxSysErrorMsgStr(unsigned long nErrCode = 0); // change even much sooner) #define wxLOG_KEY_SYS_ERROR_CODE "wx.sys_error" -#define wxLogSysError \ - if ( !wxLog::IsLevelEnabled(wxLOG_Error, wxLOG_COMPONENT) ) \ - {} \ - else \ - wxMAKE_LOGGER(Error).MaybeStore(wxLOG_KEY_SYS_ERROR_CODE, \ - wxSysErrorCode()).Log +#define wxLogSysError \ + wxDO_LOG_IF_ENABLED_WITH_FUNC(Error, MaybeStore(wxLOG_KEY_SYS_ERROR_CODE, \ + wxSysErrorCode()).Log) // unfortunately we can't have overloaded macros so we can't define versions // both with and without error code argument and have to rely on LogV() @@ -1350,11 +1333,8 @@ WXDLLIMPEXP_BASE wxString wxSysErrorMsgStr(unsigned long nErrCode = 0); // specify the frame to which the message should go #define wxLOG_KEY_FRAME "wx.frame" - #define wxLogStatus \ - if ( !wxLog::IsLevelEnabled(wxLOG_Status, wxLOG_COMPONENT) ) \ - {} \ - else \ - wxMAKE_LOGGER(Status).MaybeStore(wxLOG_KEY_FRAME).Log + #define wxLogStatus \ + wxDO_LOG_IF_ENABLED_WITH_FUNC(Status, MaybeStore(wxLOG_KEY_FRAME).Log) #define wxVLogStatus \ wxMAKE_LOGGER(Status).MaybeStore(wxLOG_KEY_FRAME).LogV @@ -1451,16 +1431,8 @@ public: #endif // wxUSE_LOG_DEBUG/!wxUSE_LOG_DEBUG #if wxUSE_LOG_TRACE - #define wxLogTrace \ - if ( !wxLog::IsLevelEnabled(wxLOG_Trace, wxLOG_COMPONENT) ) \ - {} \ - else \ - wxMAKE_LOGGER(Trace).LogTrace - #define wxVLogTrace \ - if ( !wxLog::IsLevelEnabled(wxLOG_Trace, wxLOG_COMPONENT) ) \ - {} \ - else \ - wxMAKE_LOGGER(Trace).LogVTrace + #define wxLogTrace wxDO_LOG_IF_ENABLED_WITH_FUNC(Trace, LogTrace) + #define wxVLogTrace wxDO_LOG_IF_ENABLED_WITH_FUNC(Trace, LogVTrace) #else // !wxUSE_LOG_TRACE #define wxVLogTrace(mask, fmt, valist) wxLogNop() diff --git a/tests/log/logtest.cpp b/tests/log/logtest.cpp index 5529fe1d40..7a39c1331c 100644 --- a/tests/log/logtest.cpp +++ b/tests/log/logtest.cpp @@ -394,4 +394,67 @@ void LogTestCase::NoWarnings() CPPUNIT_ASSERT_EQUAL( "If", m_log->GetLog(wxLOG_Error) ); } +// The following two functions (v, macroCompilabilityTest) are not run by +// any test, and their purpose is merely to guarantee that the wx(V)LogXXX +// macros compile without 'dangling else' warnings. +#if defined(__clang__) || wxCHECK_GCC_VERSION(4, 6) + // gcc 7 split -Wdangling-else from the much older -Wparentheses, so use + // the new warning if it's available or the old one otherwise. + #if wxCHECK_GCC_VERSION(7, 0) + #pragma GCC diagnostic error "-Wdangling-else" + #else + #pragma GCC diagnostic error "-Wparentheses" + #endif +#endif + +static void v(int x, ...) +{ + va_list list; + va_start(list, x); + + if (true) + wxVLogGeneric(Info, "vhello generic %d", list); + if (true) + wxVLogTrace(wxTRACE_Messages, "vhello trace %d", list); + if (true) + wxVLogError("vhello error %d", list); + if (true) + wxVLogMessage("vhello message %d", list); + if (true) + wxVLogVerbose("vhello verbose %d", list); + if (true) + wxVLogWarning("vhello warning %d", list); + if (true) + wxVLogFatalError("vhello fatal %d", list); + if (true) + wxVLogSysError("vhello syserror %d", list); + if (true) + wxVLogDebug("vhello debug %d", list); + + va_end(list); +} + +void macroCompilabilityTest() +{ + v(123, 456); + if (true) + wxLogGeneric(wxLOG_Info, "hello generic %d", 42); + if (true) + wxLogTrace(wxTRACE_Messages, "hello trace %d", 42); + if (true) + wxLogError("hello error %d", 42); + if (true) + wxLogMessage("hello message %d", 42); + if (true) + wxLogVerbose("hello verbose %d", 42); + if (true) + wxLogWarning("hello warning %d", 42); + if (true) + wxLogFatalError("hello fatal %d", 42); + if (true) + wxLogSysError("hello syserror %d", 42); + if (true) + wxLogDebug("hello debug %d", 42); +} + #endif // wxUSE_LOG