Refactor MFC message maps to include fully qualified member function pointers

Question

I have a codebase where MFC message maps are written in this form:

BEGIN_MESSAGE_MAP(SomeForm, BaseForm)
    ON_COMMAND(CID_ButtonAction, OnButtonAction)
END_MESSAGE_MAP()

This compiles just fine in MSVC. When I want to compile the same code in Clang I get a call to non-static member function without an object argument error because OnButtonAction is not a correct form for specifying a member function pointer. The code can be easily fixed:

    ON_COMMAND(CID_ButtonAction, &SomeForm::OnButtonAction)

or we can use ThisClass typedef from the BEGIN_MESSAGE_MAP() macro:

    ON_COMMAND(CID_ButtonAction, &ThisClass::OnButtonAction)

So far so good...the only problem is that I have hundreds of these message map entries in a lot of separate files. Is there any tool that can fix this? Some obscure Visual Studio magic? Or is it possible to use replacement via regex here?

The only supported compiler to compile MFC code is Microsoft's compiler, that comes with Visual Studio matching your MFC version. Other compilers are not supported. Even if you fix this issue, there will be so many more to fix down the road. And if things do go wrong, you're on your own. No support from Microsoft. — IInspectable, Jul 16 '18 at 13:59
My motivation here is to be able to use clang-tidy via Clang Power Tools plugin for VS and I need the code to compile for that - nothing else. I will not use clang-compiled binaries in production so there's no risk involved/support needed. Besides this fix will actually improve the quality of code making it more standard-compliant. — pablo285, Jul 16 '18 at 14:10
You should ask how to use clang-tidy with MFC then. This question is asking how to compile MFC code with Clang. Those are different questions, and they may have different answers. — IInspectable, Jul 16 '18 at 14:13
I know about a workaround for that - redefining all the MFC macros to nil when __clang__ is defined but I don't want to go that way. While using clang-tidy with MFC code is a major motivation I also want to be able to compile and link under clang if possible and this is a major hurdle on my road there. — pablo285, Jul 16 '18 at 14:17
So you *are* trying to compile MFC with Clang after all. That is simply not supported, and I'm not even sure, it is legal. You'll have to dig through the EULA to verify. — IInspectable, Jul 16 '18 at 14:18
Yes. I am. You just proved something that I stated in the original question. How is this constructive? Eula does not put any restrictions on how you compile redistrubutable code like MFC. I think I stated my question clearly and concisely, can we get back go it? — pablo285, Jul 16 '18 at 17:48
Er, ok. Just trying to understand the problem... Visual Studio doesn't provide the feature you need, not that I would know of, anyway. I'm not aware of any tools, that do what you want. That doesn't mean, there aren't any. I'm not aware of a regex that can parse C++ either. Given the very confined problem, though, you should be able to come up with a regex to run against your code to transform it to become Clang-compatible. Oddly enough, this is likely the least constructive comment on this question. — IInspectable, Jul 16 '18 at 18:17
Please include the actual error text in your posted question. — Richard Chambers, Jul 16 '18 at 19:34

pablo285 · Accepted Answer · 2019-07-19T12:03:51.023

In the end I came up with a sed command that I ran from MinGW:

sed -b -i -re '/^BEGIN_MESSAGE_MAP/,/^END_MESSAGE_MAP/{/(BEGIN_MESSAGE_MAP|\/\/)/!s/(.*),\s{0,}/\1, \&ThisClass::/;}' *.cpp

To explain what it does:

-b treat files as binary (optional, to keep line endings in Windows)*
-re support extended regular expressions
-i in-place substitution
/^BEGIN_MESSAGE_MAP/,/^END_MESSAGE_MAP/ match only text between these two strings
/!s substitution command that will ignore whatever you match before it
/\(BEGIN_MESSAGE_MAP\|\/\/\)/ matches line beginnings to ignore (either the first line of the message map or commented-out lines)
/(.*),\s{0,}/\1, \&ThisClass::/ substitutes the last comma on a line followed by 0+ whitespaces with , &ThisClass::

Sample input:

BEGIN_MESSAGE_MAP(SomeForm, BaseForm)
    ON_COMMAND(CID_ButtonAction, OnButtonAction)
    ON_NOTIFY_EX(CID_Notify, 0, OnNotify)
END_MESSAGE_MAP()

Output:

BEGIN_MESSAGE_MAP(SomeForm, BaseForm)
    ON_COMMAND(CID_ButtonAction, &ThisClass::OnButtonAction)
    ON_NOTIFY_EX(CID_Notify, 0, &ThisClass::OnNotify)
END_MESSAGE_MAP()

This worked nicely, for ~500 files I only had to make two manual adjustments where the class method membership notation was already used. The sed command could be adjusted to account for this (e.g. check if the last comma on the line was followed by &) but this was just good enough for my purposes.

EDIT - added -b option. This treats the files as binary. On Windows this prevents replacing original newline characters by Unix ones - without this option enabled the git diff for any processed file will look like the whole file has been deleted and added again.

Richard Chambers · Answer 2 · 2018-07-17T15:11:25.860

The error message is a bit strange and I suppose it has to do with a difference between Visual Studio and CLANG so far as processing the source.

The compiler I have handy is Visual Studio 2005 and I have an MFC application I am working on so the MFC source for Visual Studio 2005 is handy. I took a quick look at Visual Studio 2015 with the same solution and it appears the MFC header files are similar. So I am going to base this on Visual Studio 2005 MFC.

The ON_COMMAND() macro located in afxmsg_.h is defined as the following:

#define ON_COMMAND(id, memberFxn) \
    { WM_COMMAND, CN_COMMAND, (WORD)id, (WORD)id, AfxSigCmd_v, \
        static_cast<AFX_PMSG> (memberFxn) },
        // ON_COMMAND(id, OnBar) is the same as
        //   ON_CONTROL(0, id, OnBar) or ON_BN_CLICKED(0, id, OnBar)

And AFX_PMSG is defined in the file afxwin.h as:

// pointer to afx_msg member function
#ifndef AFX_MSG_CALL
#define AFX_MSG_CALL
#endif
typedef void (AFX_MSG_CALL CCmdTarget::*AFX_PMSG)(void);

The class CCmdTarget is a base class from which is derived other classes such as CWnd and CWinThreadand other MFC classes that use a message map.

So the ON_COMMAND() macro is using static_cast<> to what should be a base class of the window or thread target. Perhaps someone else, more knowledgable can provide an actual explanation as to what the compiler is doing and how the C++ language specification would treat this construct.

However on a more practical note, what I suggest is that you write your own version of the ON_COMMAND() macro and insert this version in the StdAfx.h file that is in each project of your solution. I picked the StdAfx.h file since there is only one per project and it is a central point where a single modification can affect multiple compile units.

At the bottom of the file after all the various includes and before the #endif that closes the test for the header file already included, add the following lines of source.

#undef ON_COMMAND

#define ON_COMMAND(id, memberFxn) \
    { WM_COMMAND, CN_COMMAND, (WORD)id, (WORD)id, AfxSigCmd_v, \
        static_cast<AFX_PMSG> (&ThisClass :: memberFxn) },
        // ON_COMMAND(id, OnBar) is the same as
        //   ON_CONTROL(0, id, OnBar) or ON_BN_CLICKED(0, id, OnBar)

This does two things.

First of all it undefines the current definition of the ON_COMMAND() macro so that you can replace it with your own.

Secondly it uses the class method membership notation for the method pointer. I am unable to test with CLANG however it should do the same source text substitution as you did by hand which you say works.

ON_COMMAND(CID_ButtonAction, &SomeForm::OnButtonAction)

ThisClass is a typedef for the class specified in the BEGIN_MESSAGE_MAP() directive (e.g. BEGIN_MESSAGE_MAP(CFrameworkWnd, CWin)) and is generated by the BEGIN_MESSAGE_MAP() macro which looks like:

#define BEGIN_MESSAGE_MAP(theClass, baseClass) \
    PTM_WARNING_DISABLE \
    const AFX_MSGMAP* theClass::GetMessageMap() const \
        { return GetThisMessageMap(); } \
    const AFX_MSGMAP* PASCAL theClass::GetThisMessageMap() \
    { \
        typedef theClass ThisClass;                        \
        typedef baseClass TheBaseClass;                    \
        static const AFX_MSGMAP_ENTRY _messageEntries[] =  \
        {

I tested this approach with Visual Studio and everything compiles just fine and it works with Visual Studio 2005.

Please note there may be other message map macros which may require a similar workaround as the use of the static_cast<AFX_PMSG> seems to be pretty common in most of the message map macros.

A curious difference

Looking into this, one curious difference in the various macros in afxmsg_.h is an entire set of macros that use the class method pointer notation. An example is the following:

#define ON_WM_PAINT() \
    { WM_PAINT, 0, 0, 0, AfxSig_vv, \
        (AFX_PMSG)(AFX_PMSGW) \
        (static_cast< void (AFX_MSG_CALL CWnd::*)(void) > ( &ThisClass :: OnPaint)) },

Looking at some of the specific event macros, it appears that they reuse the ON_CONTROL() macro so replacing that macro in addition to the ON_COMMAND() macro would ripple down through the set of control specific MFC macros.

// Combo Box Notification Codes
#define ON_CBN_ERRSPACE(id, memberFxn) \
    ON_CONTROL(CBN_ERRSPACE, id, memberFxn)

A summation

Using this approach of overriding the default macros with your own version, it appears that the include file afxmsg_.h contains a list of the what would need to change. It also appears that there are two sets of MFC macros that would need a replacement version, ones near the top of the file (beginning with ON_COMMAND()) and a few macros near the bottom of the include file afxmsg_.h.

For instance the ON_MESSAGE() macro would need a change to:

// for Windows messages
#define ON_MESSAGE(message, memberFxn) \
    { message, 0, 0, 0, AfxSig_lwl, \
        (AFX_PMSG)(AFX_PMSGW) \
        (static_cast< LRESULT (AFX_MSG_CALL CWnd::*)(WPARAM, LPARAM) > \
        (&ThisClass :: memberFxn)) },

I wonder why there is a mix of styles (possibly due to different people adding new macros over the years and not bothering to change the existing ones?). I am curious why this has not been addressed sometime during the last two decades as MFC dates from at least Visual Studio 6.x and there would have been opportunities to make the macros uniform. For instance the release of Visual Studio 2005 would have been a good time. Perhaps there was a concern for backwards compatibility with that huge Visual Studio 6.x MFC code base?

And now I know why the tailored, specific static_cast<>. It allows for the detection of a class method with the wrong or non-matching interface signature with a compilation error. So the C-style cast is to make things right with the definition for the function pointer in AFX_MSGMAP_ENTRY and the static_cast<> is to catch programmer errors due to a defective interface by issuing a compiler error if the method interface differs from what is expected.

This would work for Clang. The problem here is that my message maps don't use only ON_COMMAND but another ~30 macros that I would have to rewrite in this way. Clang compilation would depend on a modified `afxmsg_.h` which has several drawbacks - portability to other machines and broken compilation whenever MFC updates. Microsoft themselves recommend using fully qualified member function names in message map macros so I'll try to come up with a regex to fix it properly. Nice answer though, thanks. — pablo285, Jul 17 '18 at 06:15
@pablo285 I am pretty sure that Microsoft is not going to make an MFC update in this area that would break decades of source code going back to the 1990s. I have MFC code originally written with Visual Studio 6.x that compiles fine with Visual Studio 2017 so they certainly seem to make an effort on backwards compatibility in areas that do not involve C++ standards. I wonder if VS 6.X allowed class method pointer so Microsoft has had to allow it ever since? There are the /Za and the /Zc Visual Studio C++ compiler options that modify VS compiler behavior. — Richard Chambers, Jul 17 '18 at 17:18
My thoughts on this: I am going to have a helper macro (`MSGMAP_FIXUP_METHOD`?) at a central header file, to make sure to have a reliable, documented *reference* to *reliably* inherently have *all* people *understand* what this *crucial compatibility fix issue* is about; rather than using `CSomeClass` (`&CSomeClass::`) (hard-coding!!), fixups should be using the pre-supplied typedef (`ThisClass`) (as mentioned in this topic) - thereby also preventing `warning : unused typedef 'ThisClass' [-Wunused-local-typedef]`; clang `-fms-extensions` does not make ClangCL 12.0.0.0 tolerate it — LinuxDev, Mar 09 '23 at 13:27

Refactor MFC message maps to include fully qualified member function pointers

2 Answers2