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

Suppressed MSVC++ warning C5054 #2983

Closed
wants to merge 1 commit into from
Closed

Suppressed MSVC++ warning C5054 #2983

wants to merge 1 commit into from

Conversation

matt77hias
Copy link
Contributor

warning C5054: operator '|': deprecated between enumerations of different types

warning C5054: operator '|': deprecated between enumerations of different types
@ocornut
Copy link
Owner

ocornut commented Jan 16, 2020

Hmm, this is going to get really annoying quickly, if we can't inherit enums this is forcing either a lot of casts either putting the private enums in the public space. I suspect it may be easier to disable at least for our codebase (and still can be useful for user's codebase..)

Installing Visual Studio 2019 now.

@ocornut
Copy link
Owner

ocornut commented Jan 16, 2020

I can't reproduce the warning with VS2019 (16.4.3) with /W4, could you clarify how you are getting that? Thanks! (you didn't fill the requested info for PR fixing warnings)

@rokups
Copy link
Contributor

rokups commented Jan 17, 2020

Warning might come from a fact that ImGuiTreeNodeFlags_AllowItemOverlap (and others) are of ImGuiTreeNodeFlags_ type while flags and 0 are of ImGuiTreeNodeFlags type which is plain int. Given that it is strange that cast is added only for second flag and not first.

This will indeed snowball quick. Maybe better idea would be to hold off until migration to C++11. Then we can have typesafe enums.

Another slightly related detail: some tools advice to not use bitwise operations with ints. It would make sense if flag base time was unsigned instead of int (and would make clang-tidy happy).

@matt77hias
Copy link
Contributor Author

matt77hias commented Jan 17, 2020

  • imgui_widgets.cpp dear imgui, v1.75 WIP
  • Visual Studio Community 2019 16.4.3

flags |= ImGuiTreeNodeFlags_CollapsingHeader | (p_open ? ImGuiTreeNodeFlags_AllowItemOverlap | ImGuiTreeNodeFlags_ClipLabelForTrailingButton : 0);

  • flags is of type ImGuiTreeNodeFlags;
  • ImGuiTreeNodeFlags_CollapsingHeader and ImGuiTreeNodeFlags_AllowItemOverlap are enumeration values of type ImGuiTreeNodeFlags;
  • ImGuiTreeNodeFlags_ClipLabelForTrailingButton is an enumeration value of type ImGuiTreeNodeFlagsPrivate_ (which is not the same as ImGuiTreeNodeFlags). @rokups the 0 is not the culprit as this is not of an enum type, but just a plain int (which is also the underlying type of all the enums at stake).

I compile ImGui to a static library with the following command line arguments:

/JMC /permissive- /GS /W4 /Zc:wchar_t /Zi /Gm- /Od /sdl- /Zc:inline /fp:fast /D "_DEBUG" /D "_CONSOLE" /D "_UNICODE" /D "UNICODE" /errorReport:prompt /GF /WX- /Zc:forScope /RTC1 /Gd /Oi /MDd /std:c++latest /EHsc /nologo /diagnostics:column

and only get a single (i.e. no snowball effect so far) C5054. According to the docs, the combination of /W4 and /std:c++latest triggers the warning. More info can be found at: https://docs.microsoft.com/en-us/cpp/overview/cpp-conformance-improvements?view=vs-2019#binary-expressions-with-different-enum-types.

P.S.: As a general unrelated note: it is safer to use unsigned instead of signed underlying types for enums (implicit default is int) used in bit-wise arithmetic (especially if shifts are involved at some point).

@ocornut
Copy link
Owner

ocornut commented Jan 17, 2020

As a general unrelated note: it is safer to use unsigned instead of signed underlying types

I was aiming to do that when switching to C++11 (probably soon in 2020), but you are right we should be able to do it now. I have a stash for it, I'll see if it gets me any warning on some platform..

Ignoring warning C5054 for now via 2478dbf as this is bound to happen in many other places even we luckied out now with only one. Thansk for the report!

Unfortunately MSVC doesn't seem to have a __has_warning equivalent so we have to manually test for a specific version (looks like those people hate the idea of writing code that works on more than one version of the compiler..).

@ocornut ocornut closed this Jan 17, 2020
ocornut added a commit that referenced this pull request Jan 17, 2020
…istency with other backends. (#2976)

Even if realistically it is difficult to make good use of under Windows.
+ Style editor: Use a more explicit form of RadioButton() to avoid being depending on underlying flags type. (#2983)
sergeyn pushed a commit to sergeyn/imgui that referenced this pull request Mar 30, 2020
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 this pull request may close these issues.

3 participants