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

MSVC warning C4946 ("reinterpret_cast used between related classes") compiling json.hpp #1502

Closed
Mark-Dunning opened this issue Mar 6, 2019 · 7 comments
Assignees
Labels
platform: visual studio related to MSVC release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@Mark-Dunning
Copy link

  • What is the issue you have?

C4946 compiler warnings are generated when compiling json.hpp with MSVC. Note that this is an off-by-default warning but it is quite useful to detect redundant use of reinterpret_cast.

In this case it looks like the warnings are correct and the reinterpret_casts are unnecessary.

  • Please describe the steps to reproduce the issue. Can you provide a small but working code example?

Just compile json.hpp with C4946 warnings enabled:

> cl /TP /EHsc /we4946 json.hpp

Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27027.1 for x86
Copyright (C) Microsoft Corporation.  All rights reserved.

json.hpp
json.hpp(4755): error C4946: reinterpret_cast used between related classes: 'nlohmann::detail::exception' and 'nlohmann::detail::other_error'
...
  • What is the expected behavior?

No warnings.

  • And what is the actual behavior instead?

Warnings are generated.

Microsoft Visual Studio 2017 15.9.8 on Windows 10 1809.

  • Did you use a released version of the library or the version from the develop branch?

The 3.5.0 release.

n/a

@nlohmann
Copy link
Owner

I do not understand the warning, because other_error is inheriting from exception. Could this be a false positive?

@gregmarr
Copy link
Contributor

I think it's saying use static_cast instead.

@Mark-Dunning
Copy link
Author

Yes, that's right. The idea is that reinterpret_cast is a more 'dangerous' cast than a static_cast and should be reserved for cases when you really need it (e.g. casting a pointer to an intptr_t). It's also less descriptive of your intentions (not that static_cast is particularly great).

From https://en.wikipedia.org/wiki/Static_cast

The static_cast operator can be used for operations such as: ...
Converting a pointer of a base class to a pointer of a nonvirtual derived class,

@nlohmann
Copy link
Owner

Thanks, good point!

@nlohmann nlohmann self-assigned this Mar 11, 2019
nlohmann added a commit that referenced this issue Mar 11, 2019
@nlohmann nlohmann added the platform: visual studio related to MSVC label Mar 11, 2019
@nlohmann nlohmann added this to the Release 3.6.0 milestone Mar 11, 2019
@nlohmann
Copy link
Owner

@Mark-Dunning Could you please check if the issue is fixed in the latest develop branch?

@Mark-Dunning
Copy link
Author

Yes, I've just tried the latest 'develop' branch and the warning is no longer raised. Thanks very much!

@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Mar 11, 2019
@nlohmann
Copy link
Owner

Thanks for reporting and checking!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: visual studio related to MSVC release item: 🔨 further change solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

3 participants