Why does this function sometimes behave incorrectly when I use gsl::not_null?

Question

I wondered if anyone could shed some light on the root cause of sporadic behaviour in my application. This is the function so far which behaves correctly:

template<typename GetterFunction, typename SetterFunction>
void CWeekendMeetingDlg::OnSwapWith(GetterFunction&& getter,
    SetterFunction&& setter,
    UINT nID, UINT nReplacementCtrlID, UINT nReplacementDescID)
{
    // We need to locate the name for this assignment
    const auto iter = m_mapSwapWeekendAssignments.find(nID);

    if (iter != m_mapSwapWeekendAssignments.end())
    {
        // We found it
        //const gsl::not_null<CChristianLifeMinistryEntry*> pReplacement = m_mapSwapWeekendAssignments[nID];
        CChristianLifeMinistryEntry* pReplacement = m_mapSwapWeekendAssignments[nID];
        CString strExistingName, strReplacementName;
        CString strExistingDescription, strReplacementDescription;
        GetDlgItemText(m_iSwapAssignmentSourceID, strExistingName);

        if (m_iSwapAssignmentSourceDescID == IDS_STR_HOST)
        {
            CChristianLifeMinistryLabelsDlg dlgLabels(this);
            strExistingDescription = dlgLabels.GetTrimmedHostLabel();
        }
        else if (m_iSwapAssignmentSourceDescID == IDS_STR_COHOST)
        {
            CChristianLifeMinistryLabelsDlg dlgLabels(this);
            strExistingDescription = dlgLabels.GetTrimmedCoHostLabel();
        }
        else if (m_iSwapAssignmentSourceDescID == IDS_STR_ZOOM_ATTENDANT)
        {
            CChristianLifeMinistryLabelsDlg dlgLabels(this);
            strExistingDescription = dlgLabels.GetTrimmedZoomAttendantLabel();
        }
        else
            strExistingDescription.LoadString(m_iSwapAssignmentSourceDescID);

        if (nReplacementDescID == IDS_STR_HOST)
        {
            CChristianLifeMinistryLabelsDlg dlgLabels(this);
            strReplacementDescription = dlgLabels.GetTrimmedHostLabel();
        }
        else if (nReplacementDescID == IDS_STR_COHOST)
        {
            CChristianLifeMinistryLabelsDlg dlgLabels(this);
            strReplacementDescription = dlgLabels.GetTrimmedCoHostLabel();
        }
        else if (nReplacementDescID == IDS_STR_ZOOM_ATTENDANT)
        {
            CChristianLifeMinistryLabelsDlg dlgLabels(this);
            strReplacementDescription = dlgLabels.GetTrimmedZoomAttendantLabel();
        }
        else
            strReplacementDescription.LoadString(nReplacementDescID);

        CString strPrompt;
        CString strExistingDate = m_pCurrentEntry->GetMeetingDateAsString();
        CString strReplacementDate = pReplacement->GetMeetingDateAsString();
        strReplacementName = getter(pReplacement);

        strPrompt.FormatMessage(IDS_TPL_SWAP_CONFIRMATION,
            (LPCTSTR)strExistingDate,
            (LPCTSTR)strExistingName,
            (LPCTSTR)strExistingDescription,
            (LPCTSTR)strReplacementDate,
            (LPCTSTR)strReplacementName,
            (LPCTSTR)strReplacementDescription);

        if (AfxMessageBox(strPrompt, MB_ICONQUESTION | MB_YESNO) == IDYES)
        {
            strReplacementName = getter(pReplacement);
            setter(pReplacement, strExistingName);
            if (pReplacement == m_pCurrentEntry) // Swapping assignments on same meeting!
                SetDlgItemText(nReplacementCtrlID, strExistingName);

            SetDlgItemText(m_iSwapAssignmentSourceID, strReplacementName);
            GetParent()->PostMessage(UM_SM_EDITOR_SET_MODIFIED, gsl::narrow<WPARAM>(TRUE));
        }
    }
}

The reason it behaves correctly is because I stopped using the gsl::not_null code:

//const gsl::not_null<CChristianLifeMinistryEntry*> pReplacement = m_mapSwapWeekendAssignments[nID];
CChristianLifeMinistryEntry* pReplacement = m_mapSwapWeekendAssignments[nID];

I appreciate that this is not a minimum working example, but it is tricky to supply that. This function is used to build by a context menu.

What I found was that when I used the not_null approach there were instances where the pReplacement pointer was not correct. In fact, once it went wrong, it would stay on this other pointer in the list. Have I described it well?

Anyway, when I stop using not_null it works as expected with no odd behaviour. It is always the right pReplacement pointer, every time. Why the odd behaviour when using not_null?

---

I ought to point out that the reason I tried to use gsl::not_null was to address a C26429 code analysis warning.

---

Here is an example of the calling code where I define the getter and setter:

void CWeekendMeetingDlg::OnSwapWithZoomAttendantAssignment(UINT nID)
{
    auto getter = [](gsl::not_null<CChristianLifeMinistryEntry*> pReplacement)
    {
        S_TALK_INFO sPTI = pReplacement->GetPublicTalkInfo();
        return sPTI.strZoomAttendant;
    };
    auto setter = [](gsl::not_null<CChristianLifeMinistryEntry*> pReplacement, CString& strExistingName)
    {
        S_TALK_INFO sPTI = pReplacement->GetPublicTalkInfo();
        sPTI.strZoomAttendant = strExistingName;
        pReplacement->SetPublicTalkInfo(sPTI);
    };
    CWeekendMeetingDlg::OnSwapWith(
        getter, setter, nID, IDC_COMBO_WEEKEND_ZOOM_ATTENDANT, IDS_STR_ZOOM_ATTENDANT);
}

Answer 1

It turns out that my initial thoughts about not using gsl::not_null was a red herring because the odd behaviour was still happening.

I did another close inspection of my code since I have a similar Swap Assignment context menu in another editor in the software. And I stumbled on the bug.

I needed to add the clear() call here to reset the map:

BOOL CWeekendMeetingDlg::PreTranslateMessage(MSG* pMsg)
{
    m_ToolTips.RelayEvent(pMsg);

    if (IsCTRLpressed() && pMsg->message == WM_LBUTTONDOWN)
    {
        m_mapSwapWeekendAssignments.clear();

        if (ValidateAssignmentControl(pMsg->hwnd))
        {
            DisplaySwapAssignmentsPopupMenu(pMsg->pt);
            return true;
        }
    }

    return CResizingDialog::PreTranslateMessage(pMsg);
}

When I was clicking on another control to start beginning a new swap process it was not resetting the map, so it was getting messed up and confused. Clearing the map resolves the issue.