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

Cannot compile with /GR- (RTTI disabled) and _HAS_STATIC_RTTI=0 with MSVC compiler #669

Closed
inkychris opened this issue Nov 9, 2021 · 5 comments · Fixed by #666
Closed

Comments

@inkychris
Copy link

I get the following error compiling a CLI11 example with /GR- (RTTI disabled) and also with -D_HAS_STATIC_RTTI=0:

CLI11.hpp(6763,25): error C2280: 'std::shared_ptr<CLI::ConfigBase> std::dynamic_pointer_cast<CLI::ConfigBase,CLI::Config>(const std::shared_ptr<CLI::Config> &) noexcept': attempting to reference a deleted function

The condition check for whether to use a dynamic or static cast is the issue here:

CLI11/include/CLI/App.hpp

Lines 1546 to 1550 in 8155532

#if defined(__cpp_rtti) || (defined(__GXX_RTTI) && __GXX_RTTI) || (defined(_HAS_STATIC_RTTI) && (_HAS_STATIC_RTTI == 0))
return std::dynamic_pointer_cast<ConfigBase>(config_formatter_);
#else
return std::static_pointer_cast<ConfigBase>(config_formatter_);
#endif

MSVC does not appear to modify the value of __cpp_rtti which remains 1, but rather sets _CPPRTTI instead. Visual Studio's intellisense doesn't seem to correctly interpret this value but it does in fact change depending on /GR[-].

If my understanding is correct, the condition for _HAS_STATIC_RTTI should also be checking for a value of 1, not 0.

@phlptp
Copy link
Collaborator

phlptp commented Nov 9, 2021

I think the _HAS_STATIC_RTTI is correct. If that is set you want to use the static cast, it looks like we need to expand the conditions of __cpp_rtti and possibly a similar one for _CPPRTTI

We also might want to flip the conditions a little to better reflect the different possibilities you described. Need to think a little more

@phlptp
Copy link
Collaborator

phlptp commented Nov 9, 2021

I added a potential fix in #666, not sure if you can check that, or if we should try to tweak one of the CI builds with RTTI disabled like in the issue? @henryiii thoughts?

@inkychris
Copy link
Author

I was under the impression that _HAS_STATIC_RTTI=0 defined but disabled it, although I think this STL source suggests I misunderstood this variable, it seems a little backwards to me!

https://github.com/microsoft/STL/blob/178b8406a5647507e759442960f17cd33045cdf5/stl/inc/yvals_core.h#L591-L597

Your changes seem to have fixed the issue for me and it's correctly selecting the static cast now.

@phlptp
Copy link
Collaborator

phlptp commented Nov 10, 2021

I think I have always interpreted it as if that is set it has RTTI but don't use the dynamic RTTI.

@inkychris
Copy link
Author

inkychris commented Nov 10, 2021

Yeah that STL snippet I linked seems to agree with you, since it throws an error if you both define _HAS_STATIC_RTTI and have /GR (enable RTTI). I think I need to pull those definitions out of my codebase!

Actually on reflection, I think the value of that definition is taken into consideration, as implied by both these threads:

That said, your proposed change is still behaving correctly. CLI11 should be using the static cast if _HAS_STATIC_RTTI=0. The original condition was in fact supposed to be checking for a value of 1 and not 0.

@phlptp phlptp linked a pull request Nov 12, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants