This is an archive of the discontinued LLVM Phabricator instance.

[clang][msvc] Define _HAS_STATIC_RTTI to 0, when compiling with -fno-rtti
ClosedPublic

Authored by zero9178 on Jun 6 2021, 10:42 AM.

Details

Summary

When using the -fno-rtti option of the GCC style clang++, using typeid results in an error. The MSVC STL however kindly provides a define flag called _HAS_STATIC_RTTI, which either enables or disables uses of typeid throughout the STL. By default, if undefined, it is set to 1, enabling the use of typeid.

With this patch, _HAS_STATIC_RTTI is set to 0 when -fno-rtti is specified. This way various headers of the MSVC STL like functional can be consumed without compilation failures.


Some context: I was first made aware of this define in this bug report regarding twoPhase lookup: https://bugs.chromium.org/p/chromium/issues/detail?id=996675
Back then however there was still a usage of typeid left unguarded, which has since been fixed in this commit https://github.com/microsoft/STL/issues/340.
Since that is November 2019, so a rather recent version of MSVC, this will not work in versions prior to this fix.
For compat one could maybe allow typeid(void), but that is beyond the scope of this patch either way I think.

Diff Detail

Event Timeline

zero9178 requested review of this revision.Jun 6 2021, 10:42 AM
zero9178 created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2021, 10:42 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
zero9178 updated this revision to Diff 350123.Jun 6 2021, 11:57 AM

Rebase & add comment in source code explaining the purpose of the define

Some background https://bugs.chromium.org/p/chromium/issues/detail?id=996675 : Back then I thought that MSVC switched to /Zc:twoPhase , but that turned out to not be true. So we never switched the default.

I think this patch here still makes sense, but I admit I don't have all the details paged in. Let's see if rnk or hans have opinions :)

This revision is now accepted and ready to land.Jun 7 2021, 5:38 AM
rnk accepted this revision.Jun 7 2021, 9:46 AM

Seems reasonable to me.

Here is where it is defined in MSVC's yvals_core.h for the curious:
https://github.com/microsoft/STL/blob/3cafa97eecdbfde41ea5c09126f877a7eb97f9e9/stl/inc/yvals_core.h#L569