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

Cleans up MSVC only warnings pragmas. #1013

Merged
merged 2 commits into from
Jun 25, 2024

Conversation

OmniBlade
Copy link
Contributor

Any warnings that need to be disabled at the current warning level are disabled through CMakePresets.json now. Also fixes two warnings seen at current warning level.
Obsoletes #981

Any warnings that need to be disabled at the current warning level are disabled through CMakePresets.json now.
Also fixes two warnings seen at current warning level.
@@ -196,6 +196,7 @@ unsigned sosCODECDecompressData(_SOS_COMPRESS_INFO* stream, unsigned bytes)
}
#endif
assert(0 && "Unreachable");
return 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is unreachable then why are you returning 0 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its in theory reachable while the else branch is ifdef out when compiling with NDEBUG which triggers compiler warnings. Other option is to remove the ifdef and let the else branch ride?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I would expect the compiler to not complain once assert(0 && string) = assert(0) and hence anything beyond that would fall into dead code.

But perhaps the compiler has bugs...

@OmniBlade OmniBlade merged commit 9641ffc into TheAssemblyArmada:vanilla Jun 25, 2024
17 checks passed
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.

2 participants