1

See this code:

COleDateTime CSpecialEventDlg::GetSpecialEventDate() noexcept
{
    COleDateTime    datEvent;

    if (datEvent.SetDateTime(m_datEvent.GetYear(),
            m_datEvent.GetMonth(),
            m_datEvent.GetDay(),
            0, 0, 0) != 0)
    {
        // Error

    }

    return datEvent;
}

My code analysis said I could add noexcept (which I did). But I decided to investigate a little more. I noticed that SetDateTime returns a value:

Zero if the value of this COleDateTime object was set successfully; otherwise, 1. This return value is based on the DateTimeStatus enumerated type. For more information, see the SetStatus member function.

For that latter function (SetStatus) it states:

The status parameter value is defined by the DateTimeStatus enumerated type, which is defined within the COleDateTime class. See COleDateTime::GetStatus for details.

Now, the documentation for GetStatus is documented to have a definition of:

DateTimeStatus GetStatus() const throw();

So, it throws an exception if there is an error. Therefore, I decided to look at the MFC source for SetDateTime:

inline int COleDateTime::SetDateTime(
    _In_ int nYear,
    _In_ int nMonth,
    _In_ int nDay,
    _In_ int nHour,
    _In_ int nMin,
    _In_ int nSec) throw()
{
    SYSTEMTIME st;
    ::ZeroMemory(&st, sizeof(SYSTEMTIME));

    st.wYear = WORD(nYear);
    st.wMonth = WORD(nMonth);
    st.wDay = WORD(nDay);
    st.wHour = WORD(nHour);
    st.wMinute = WORD(nMin);
    st.wSecond = WORD(nSec);

    m_status = ConvertSystemTimeToVariantTime(st) ? valid : invalid;
    return m_status;
}

It uses ConvertSystemTimeToVariantTime to set the status. That uses:

inline BOOL COleDateTime::ConvertSystemTimeToVariantTime(_In_ const SYSTEMTIME& systimeSrc)
{
    return AtlConvertSystemTimeToVariantTime(systimeSrc,&m_dt);
}

And that uses:

inline BOOL AtlConvertSystemTimeToVariantTime(
    _In_ const SYSTEMTIME& systimeSrc,
    _Out_ double* pVarDtTm)
{
    ATLENSURE(pVarDtTm!=NULL);
    //Convert using ::SystemTimeToVariantTime and store the result in pVarDtTm then
    //convert variant time back to system time and compare to original system time.
    BOOL ok = ::SystemTimeToVariantTime(const_cast<SYSTEMTIME*>(&systimeSrc), pVarDtTm);
    SYSTEMTIME sysTime;
    ::ZeroMemory(&sysTime, sizeof(SYSTEMTIME));

    ok = ok && ::VariantTimeToSystemTime(*pVarDtTm, &sysTime);
    ok = ok && (systimeSrc.wYear == sysTime.wYear &&
            systimeSrc.wMonth == sysTime.wMonth &&
            systimeSrc.wDay == sysTime.wDay &&
            systimeSrc.wHour == sysTime.wHour &&
            systimeSrc.wMinute == sysTime.wMinute &&
            systimeSrc.wSecond == sysTime.wSecond);

    return ok;
}

At this point I have got lost. In short, I am assuming that SetDateTime does not throw an exception.


I understand this much, if I decide to make a call to GetStatus inside the if clause then we do have a potential for exception to be thrown.

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
Andrew Truckle
  • 17,769
  • 16
  • 66
  • 164
  • 1
    Rather than leave my original bunch of comments, I decided to amalgamate them into an answer. Not sure if it fully addresses your concerns, but it may go *some* way to clearing your doubts. – Adrian Mole Oct 16 '21 at 21:07
  • 1
    Hi Andrew, I gave up on using COleDateTime some time ago. It is still in some of my code, but I have a date class that can eat COleDateTime while I replace it. The biggy for me is that they use a double!!?? [Leap years](https://stackoverflow.com/questions/60473361/what-is-the-right-way-to-add-a-year-to-coledatetime-taking-into-account-leap-yea)?? [the paste](https://pastebin.com/TevHYTeD) if you like. And you now have access to all the boost::gregorian goodies! – lakeweb Oct 17 '21 at 15:41
  • 1
    @lakeweb I have heavily used `COleDateTime` in my project over the years and have some bits fine tuned for languages like Polish and Maltese. So I am reluctant to change the class I use now. But thanks for the link. – Andrew Truckle Oct 17 '21 at 16:09

1 Answers1

1

All three MFC functions for which you have shown the source code (COleDateTime::SetDateTime, COleDateTime::ConvertSystemTimeToVariantTime and AtlConvertSystemTimeToVariantTime) have – according to their declarations – the potential to throw exceptions (because they have no specification to the contrary, such as noexcept).

However, that doesn't mean that they will (or are even likely to). Digging a bit further into the MFC source code, the only place I can see, in those three functions, where an exception could be thrown is in the ATLENSURE(pVarDtTm!=NULL); line (in the third function).

The ATLENSURE macro is defined as follows:

#define ATLENSURE(expr) ATLENSURE_THROW(expr, E_FAIL)

And ATLENSURE_THROW is, in turn, defined thus:

#define ATLENSURE_THROW(expr, hr)          \
do {                                       \
    int __atl_condVal=!!(expr);            \
    ATLASSUME(__atl_condVal);              \
    if(!(__atl_condVal)) AtlThrow(hr);     \
} __pragma(warning(suppress:4127)) while (0)

So, it would seem that, in your code, an exception will be thrown if expr (in the above snippets) is null (the double-bang, !! pseudo-operator makes any non-zero value into 1 and leaves a zero as zero). That expr is the result of the pVarDtTm!=NULL expression, which can only be 0 (false) if the &m_dt argument in the call in your second MFC source excerpt is itself NULL – and, as the address of a member of the class object through which it is being called, that seems improbable (if not impossible).


Another issue you have is that you appear to misunderstand what the throw() specification in the DateTimeStatus GetStatus() const throw(); declaration means. As described here, it is actually (since C++17) an alias for noexcept (or noexcept(true), to be more precise). To specify that a function can throw any type of exception, the throw(...) or noexcept(false) specifications should be used (or use no except/throw specification at all).

So, your last sentence is not really true:

I understand this much, if I decide to make a call to GetStatus inside the if clause then we do have a potential for exception to be thrown.

Because the GetStatus() function is specified explicitly as non-throwing, and you already have a call to the SetDateTime member function, which (as described above) can throw an exception (but won't, in your code).

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
  • Thanks. So for my code I should remove the noexcept keyword then …? Sounds like it. – Andrew Truckle Oct 16 '21 at 21:08
  • 1
    @Andrew Seems like it's safe to leave it, from how I understand the MFC source code and calling chain. Your code analyser appears to agree with me. – Adrian Mole Oct 16 '21 at 21:09
  • 1
    Leave it, as in, leave it in? – Andrew Truckle Oct 16 '21 at 21:15
  • 1
    Yes. Use the `noexcept` specification. In the *extremely unlikely* event that an exception is raised during a call to that function, your program will call `std::terminate` (assuming a C++17 or later standard). Furthermore, unless *every* call you make to your `GetSpecialEventDate` function is enclosed in a `try...catch` block, then the program will crash and burn, anyway. – Adrian Mole Oct 16 '21 at 21:25