Page MenuHomePhabricator

[PR52549][clang-cl] Predefine _MSVC_EXECUTION_CHARACTER_SET
ClosedPublic

Authored by zero9178 on Thu, Nov 25, 1:55 AM.

Details

Summary

Since VS 2022 17.1 MSVC predefines _MSVC_EXECUTION_CHARACTER_SET to inform the users of the execution character set defined at compile time. The value the macro expands to is a Windows Code Page Identifier which are documented here: https://docs.microsoft.com/en-us/windows/win32/intl/code-page-identifiers

As clang currently only supports UTF-8 it is defined as 65001. If clang-cl were to support a different execution character set in the future we'd have to change the value.

Fixes https://bugs.llvm.org/show_bug.cgi?id=52549

Diff Detail

Event Timeline

zero9178 requested review of this revision.Thu, Nov 25, 1:55 AM
zero9178 created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptThu, Nov 25, 1:55 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Would it be possible to move setting of this flag to somewhere else (e.g. somewhere in Driver/ToolChains/MSVC.cpp), so that it would be picked up also when building with the gcc-style driver with --target=*-windows-msvc? Overall in earlier discussions, there's been mostly positive reception to making that mode more usable even if it isn't quite as supported as actually invoking clang-cl. (For some things, like the equivalent of the cl level -MD/-MT options, there's currently nothing available in the gcc style driver - but for a feature like this, I don't see any reason why it shouldn't be available everywhere.)

zero9178 updated this revision to Diff 389698.Thu, Nov 25, 2:23 AM

Enable the macro for all MSVC targets, regardless of driver in use.

mstorsjo accepted this revision.Thu, Nov 25, 2:40 AM

Thanks! This LGTM - but maybe wait a while if someone else has something to say. (Although I guess americans are on holiday today, so there's probably not many that will see and care today.)

This revision is now accepted and ready to land.Thu, Nov 25, 2:40 AM
CaseyCarter accepted this revision.Thu, Nov 25, 3:00 AM

LGTM - thanks for the quick response!

Seems like this is causing failures in a test for clang-tidy if the triple used is an MSVC one. Let me see what I can do about it

zero9178 updated this revision to Diff 390156.Sat, Nov 27, 4:38 AM

Alternative implementation of the original patch.

Moves the defining macro from being added to the cc1 command line by the driver, to being a builtin macro. This should avoid the issues with clang-tidy.

rnk added a comment.Tue, Nov 30, 11:00 AM

Would it be possible to move setting of this flag to somewhere else (e.g. somewhere in Driver/ToolChains/MSVC.cpp), so that it would be picked up also when building with the gcc-style driver with --target=*-windows-msvc? Overall in earlier discussions, there's been mostly positive reception to making that mode more usable even if it isn't quite as supported as actually invoking clang-cl. (For some things, like the equivalent of the cl level -MD/-MT options, there's currently nothing available in the gcc style driver - but for a feature like this, I don't see any reason why it shouldn't be available everywhere.)

+1, that was going to be my suggestion.