skypjack / entt

Gaming meets modern C++ - a fast and reliable entity component system (ECS) and much more
https://github.com/skypjack/entt/wiki
MIT License
10.04k stars 880 forks source link

Implicit conversion warnings #394

Closed frimarlucas closed 4 years ago

frimarlucas commented 4 years ago

Hi, I'm trying to add EnTT to an Unreal project with C++17 enabled. When compiling, I see these warnings. Are these known, or have I done something wrong?

Thanks!

   E:\testy\Source\entt\meta\meta.hpp(562) : error C4800: Implicit conversion from 'const entt::internal::meta_type_node *const ' to bool. Possible information loss
  E:\testy\Source\entt\meta\meta.hpp(562): note: consider using explicit cast or comparison to nullptr to avoid this warning
   E:\testy\Source\entt\meta\meta.hpp(690) : error C4800: Implicit conversion from 'const entt::internal::meta_prop_node *const ' to bool. Possible information loss
  E:\testy\Source\entt\meta\meta.hpp(690): note: consider using explicit cast or comparison to nullptr to avoid this warning
   E:\testy\Source\entt\meta\meta.hpp(728) : error C4800: Implicit conversion from 'const entt::internal::meta_base_node *const ' to bool. Possible information loss
  E:\testy\Source\entt\meta\meta.hpp(728): note: consider using explicit cast or comparison to nullptr to avoid this warning
   E:\testy\Source\entt\meta\meta.hpp(763) : error C4800: Implicit conversion from 'const entt::internal::meta_conv_node *const ' to bool. Possible information loss
  E:\testy\Source\entt\meta\meta.hpp(763): note: consider using explicit cast or comparison to nullptr to avoid this warning
   E:\testy\Source\entt\meta\meta.hpp(847) : error C4800: Implicit conversion from 'const entt::internal::meta_ctor_node *const ' to bool. Possible information loss
  E:\testy\Source\entt\meta\meta.hpp(847): note: consider using explicit cast or comparison to nullptr to avoid this warning
   E:\testy\Source\entt\meta\meta.hpp(984) : error C4800: Implicit conversion from 'const entt::internal::meta_data_node *const ' to bool. Possible information loss
  E:\testy\Source\entt\meta\meta.hpp(984): note: consider using explicit cast or comparison to nullptr to avoid this warning
   E:\testy\Source\entt\meta\meta.hpp(1095) : error C4800: Implicit conversion from 'const entt::internal::meta_func_node *const ' to bool. Possible information loss
  E:\testy\Source\entt\meta\meta.hpp(1095): note: consider using explicit cast or comparison to nullptr to avoid this warning
   E:\testy\Source\entt\meta\meta.hpp(1444) : error C4800: Implicit conversion from 'const entt::internal::meta_type_node *const ' to bool. Possible information loss
  E:\testy\Source\entt\meta\meta.hpp(1444): note: consider using explicit cast or comparison to nullptr to avoid this warning
   E:\testy\Source\entt\signal\delegate.hpp(281) : error C4800: Implicit conversion from 'void (__cdecl *const )(const void *,std::tuple<void *&&>)' to bool. Possible information loss
  E:\testy\Source\entt\signal\delegate.hpp(279): note: consider using explicit cast or comparison to nullptr to avoid this warning
  E:\testy\Source\entt\signal\delegate.hpp(279): note: while compiling class template member function 'entt::delegate<void (void *)>::operator bool(void) noexcept const'
  E:\testy\Source\entt\entity\../signal/sigh.hpp(170): note: see reference to function template instantiation 'entt::delegate<void (void *)>::operator bool(void) noexcept const' being compiled
  E:\testy\Source\entt\entity\../signal/sigh.hpp(182): note: see reference to class template instantiation 'entt::delegate<void (void *)>' being compiled
skypjack commented 4 years ago

Hi there,

Well, EnTT has already been integrated with UE4 successfully. Therefore, I suspect it's due to your configuration. What compilers and what warnings/options/whatever did you enable? This is the first time I see these errors and almost all them are due to pointer-to-bool conversions, that isn't exactly something rare or arcane in C++.

frimarlucas commented 4 years ago

This is a completely fresh project, on a fresh install of Unreal 4.24.1. Literally the only things I've done are:

I also tried with the latest EnTT code as of today, and both the full src directory and the single include file. The errors also occur both when building from Unreal and from Visual Studio 2019 directly.

I'm not overly familiar with Unreal itself, but the output when compiling includes this:

Running UnrealHeaderTool "E:\testy\testy.uproject" "E:\testy\Intermediate\Build\Win64\testyEditor\Development\testyEditor.uhtmanifest" -LogCmds="loginit warning, logexit warning, logdatabase error" -Unattended -WarningsAsErrors -installed

Using Visual Studio 2019 14.24.28314 toolchain (C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\VC\Tools\MSVC\14.24.28314) and Windows 10.0.18362.0 SDK (C:\Program Files (x86)\Windows Kits\10).

I'm not really sure where else to go from here.

skypjack commented 4 years ago

The errors also occur both when building from Unreal and from Visual Studio 2019 directly.

The CI contains jobs for vs2017 and vs2019. That's why I'm asking you what your configuration is. You can verify yourself that I can't reproduce your case with a default configuration.

It would be great if you told me how to make vs trigger a warning for a pointer-to-bool conversion. Actually, it would be even better if you found the property that makes it happen and get rid of this. :smile:

In the meantime, drop this one: -WarningsAsErrors.

skypjack commented 4 years ago

Hi there, any news? As it stands, I cannot do much on this issue. I can't reproduce the problem without more details unfortunately and the issue is hanging at the moment.

indianakernick commented 4 years ago

C4800 is a level 4 warning (level 3 in VS2015 and earlier) so you must be using /W4 or /Wall. You just need to dial back the warning level by using /W3 or the VS2019 GUI.

skypjack commented 4 years ago

Or use /W4 without -WarningsAsErrors.

frimarlucas commented 4 years ago

Hi, after some searching online yesterday I suspect that it might be related to having a certain combination of VS17/19 and various Windows 10 SDKs installed, although I'm not in a position at work to uninstall things to verify.

Unreal doesn't seem to expose any actual compiler options (but I'm not familiar with it, so it might be buried somewhere deep), and the compilation process goes through a closed executable, so I'm unsure of the final flags and suchlike.

I would like to test it at home this week if I can find the time. The repro was literally just: open Unreal 2.42.1, create a new project, copy the EnTT files, add the following code to enable C++17:

x.Build.cs
PCHUsage = PCHUsageMode.NoSharedPCHs;
PrivatePCHHeaderFile = "PCH.h";
CppStandard = CppStandardVersion.Cpp17;

x.cpp
#include <entt/entt.hpp>

PCH.h
#pragma once
skypjack commented 4 years ago

I would like to test it at home this week if I can find the time.

Take your time. I'll keep the issue open for a while. :+1: Thanks for the feedback.

frimarlucas commented 4 years ago

I tested using a combination of different Window's 10 SDK versions at home, where I don't have VS17 installed, with a completely fresh install of Unreal. Unfortunately, the warnings still appeared.

Are you 100% sure that your tests log warnings?

skypjack commented 4 years ago

Are you 100% sure that your tests log warnings?

It's not a problem of logging warnings. The problem is that you're using /W4 and -WarningsAsErrors. /W4 returns also informative messages. In particular, it returns also foolish things like that you're making a pointer-to-bool conversion (something pretty common indeed). If you combine it with -WarningsAsErrors, nothing good will happen.

Who set them? Because that stuff is the source of your problems. Either use /W1 or /W2 or don't treat warnings as error. I won't get rid of pointer-to-bool conversions because of this, it doesn't make sense to put useless static_casts all around just to suppress a bunch of informative warnings.

frimarlucas commented 4 years ago

Yep, I guess my main point with this issue is that a completely fresh Unreal project with EnTT cannot compile, and simply hiding warnings should always be the very last thing to do to be rid of them (if I can even find a way to do so in Unreal...).

I don't think there's much more to say here, so I leave the decision in your hands :)

skypjack commented 4 years ago

simply hiding warnings should always be the very last thing to do

I'm not saying to hide them. I'm saying to treat them as warnings rather than as hard errors, especially if you turn on informative logs. A compiler that tells you that you're performing a pointer-to-bool conversion is debatable but let's say it's ok for you. This conversion has nothing wrong though, I'm not going to turn all them into something different just because we cannot compile something that treats them as errors. This is valid C++ and pretty common too. Almost all the non-toy projects do this kind of conversions. What do you suggest exactly? To convert all these:

return ptr;

Into this?

return static_cast<bool>(ptr);

This won't happen, it doesn't make sense to me. As I said, just don't treat them as errors.

As a side note, EnTT has been used more than once successfully with UE4. Join the gitter channel if you've doubts, there are users there that can help you on this aspect. Probably the error is trivial and there exists an easy way to make your project eat it. :wink:

Innokentiy-Alaytsev commented 4 years ago

Documentation for MSVC warning control flags. /W4 may be of use for reporting warnings that may appear when using EnTT with Unreal Engine.

Tyyppi77 commented 4 years ago

Reproducible on MSVC with /w14800 and /WX.

skypjack commented 4 years ago

Let's tackle this in. :+1:

skypjack commented 4 years ago

@frimarlucas we discovered that /w14800 is the root cause of these warnings. I added it to the build system and made what is needed to suppress them all. I've no idea if this is enough for UE4 because maybe they use also some other specifiers of which I'm not aware and therefore some other warnings could trigger.

Please, let me know if you give it another run. Thank you very much.

ryobg commented 11 months ago

This is from an Unreal Engine 5.2 project. Just including the basic entt::registry r; caused the following compilation errors:

EnTT\entt\meta\meta.hpp(1322): error C4800: Implicit conversion from 'const void *' to bool. Possible information loss
EnTT\entt\meta\meta.hpp(1322): note: consider using explicit cast or comparison to nullptr to avoid this warning
EnTT\entt\meta\meta.hpp(1331): error C4800: Implicit conversion from 'unsigned int' to bool. Possible information loss
EnTT\entt\meta\meta.hpp(1331): note: consider using explicit cast or comparison to 0 to avoid this warning

I know I can add a flag or pragma warnings, but it seems good to know that maybe more strict systems will complain even today. These two were the only ones so far.

skypjack commented 11 months ago

can_cast and can_convert @ryobg right? Thanks for the heads-up. I'm fixing them on the working branch. 👍

ryobg commented 11 months ago

can_cast and can_convert @ryobg right? Thanks for the heads-up. I'm fixing them on the working branch. 👍

Yep! Thank you for your effort 👍