Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std.ixx: Defining _HAS_STATIC_RTTI=0 and importing std fails to compile due to any. #3114

Closed
tylerbrawl opened this issue Sep 19, 2022 · 2 comments · Fixed by #3115
Closed
Labels
bug Something isn't working fixed Something works now, yay! modules C++23 modules, C++20 header units

Comments

@tylerbrawl
Copy link
Contributor

Describe the bug
(Very) recently, #2930 was merged into main to add the std and std.compat module interface units from P2465R3. I went to go test this out in a forked repository to see how it works out. Typically, I define _HAS_STATIC_RTTI=0 as a define for the projects which I work on. However, this results in a compilation error due to <any> requiring that _HAS_STATIC_RTTI != 0.

Command-line test case
[Assume that the current directory contains std.ixx.]

C:\Temp>cl /EHsc /W4 /WX /GR- /D "_HAS_STATIC_RTTI=0" /permissive- /std:c++latest .\std.ixx
Microsoft (R) C/C++ Optimizing Compiler Version 19.34.31823.3 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

/std:c++latest is provided as a preview of language features from the latest C++
working draft, and we're eager to hear about bugs and suggestions for improvements.
However, note that these features are provided as-is without support, and subject
to changes or removal as the working draft evolves. See
https://go.microsoft.com/fwlink/?linkid=2045807 for details.

std.ixx
C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.34.31823\include\any(22): fatal error C1189: #error:  class any requires static RTTI.

Expected behavior
According to the original paper, there are no exceptions as to which declarations from the std namespace are actually exported by the std module; thus, the current behavior is technically correct. In practice, however, I can see this causing a lot of issues in real-world projects.

The only thing which I can think of for working around this is to conditionally #include <any> within std.ixx depending on the value of _HAS_STATIC_RTTI. This would technically be non-conformant, but _HAS_STATIC_RTTI is already a non-standard macro. Besides, if the user already defined _HAS_STATIC_RTTI=0, then it's pretty safe to assume that they weren't going to use anything in <any>, anyways.

I didn't see any mention of this in #3108, but if this issue was already discussed elsewhere, then please feel free to close this bug report.

STL version
This issue was found in a fork from the STL repo's main branch at commit 2f03bdf. Since the issue was reported early enough, current users of the MSVC STL are not affected by this bug. With that said, this still seems like a fairly urgent issue.

@CaseyCarter CaseyCarter added the bug Something isn't working label Sep 20, 2022
@CaseyCarter
Copy link
Contributor

My initial reaction is to close this as unsupported (since _HAS_STATIC_RTTI is itself not officially supported) but I'm swayed by your argument that support is as simple as not including <any> when building the module with _HAS_STATIC_RTTI defined to a nonzero value. I think we could accept a PR making that simple change without undue maintenance burden.

@frederick-vs-ja
Copy link
Contributor

Strangely, when _HAS_STATIC_RTTI is defined as 0, #include <any> just triggers a warning in C++14 mode, but is erroneous in C++17 and later modes. I think the we should make the latter case effectless (except for warnings).
IIUC such consistency fix will also fix this issue.

(BTW, it's already non-conformant to disable static RTTI by any means since it's mandatory in the Standard...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed Something works now, yay! modules C++23 modules, C++20 header units
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants