This is an archive of the discontinued LLVM Phabricator instance.

[MinGW] Predefine UNICODE if -municode is specified during compilation
ClosedPublic

Authored by mstorsjo on Aug 2 2018, 12:51 PM.

Diff Detail

Event Timeline

mstorsjo created this revision.Aug 2 2018, 12:51 PM
rnk added a comment.Aug 2 2018, 12:55 PM

Does this do anything other than -DUNICODE? Maybe just translate it at the driver level and skip the -cc1 flag?

In D50199#1186164, @rnk wrote:

Does this do anything other than -DUNICODE? Maybe just translate it at the driver level and skip the -cc1 flag?

Yes, that also works. Initially I looked at where to place that in lib/Driver/ToolChains/MinGW.cpp, but I see that it should be in lib/Driver/ToolChains/Clang.cpp (I presume?).

mstorsjo updated this revision to Diff 158827.Aug 2 2018, 1:12 PM

Simplified the patch to just handle the flag within the driver, without making it a flag to cc1.

pirama added a comment.Aug 2 2018, 1:58 PM
In D50199#1186164, @rnk wrote:

Does this do anything other than -DUNICODE? Maybe just translate it at the driver level and skip the -cc1 flag?

It seems odd to include predefined macros at the driver, which AFAIK is just a bridge to the frontend's command-line interface. OTOH, there is other precedence for doing this - during clang-cl's argument processing. So I don't have a strong opinion here.

Are you sure this shouldn't also define _UNICODE? looks Hmm... GCC doesn't define it. I wonder if we should anyway. A user who set -municode may also be using <tchar.h>, and thus may want those macros set to their Unicode counterparts.

I actually believe this is supposed to have one other effect: it sets the entry point to wmainCRTStartup()/wWinMainCRTStartup(), making the user entry point wmain()/wWinMain(), which take wide arguments:

int wmain(int argc, wchar_t **argv);
int WINAPI wWinMain(HINSTANCE hinst, HINSTANCE hinstPrev, LPWSTR pwszCmdLine, int nCmdShow);

But that's a link-time effect. I believe it has no bearing on @rnk's comment.

...Actually, that suggests that we may want to warn if the user defines main() or WinMain() instead of their Unicode counterparts when -municode is given--which means we may want to pass it to the frontend after all.

I actually believe this is supposed to have one other effect: it sets the entry point to wmainCRTStartup()/wWinMainCRTStartup(), making the user entry point wmain()/wWinMain(), which take wide arguments:

It already does this; lld (and I guess ld.bfd as well) detect the entry point based on what symbols are available. But this flag switches between passing crt2.o and crt2u.o to the linker, essentially forcing an undefined reference to either main or wmain.

rnk accepted this revision.Aug 6 2018, 11:01 AM

lgtm, thanks!

This revision is now accepted and ready to land.Aug 6 2018, 11:01 AM
This revision was automatically updated to reflect the committed changes.