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

Fix two MSVC warnings #160

Merged
merged 3 commits into from
Feb 19, 2022
Merged

Conversation

chausner
Copy link
Contributor

@chausner chausner commented Dec 15, 2021

This fixes the following warning with MSVC:

...\src\time_postprocessor.cpp(7,9): warning C4068: Unknown pragma "GCC".

Before this change, this pragma was active when compiling with clang. I am not sure if this was on purpose and whether it has any effect on clang. If yes, I can change the condition to:

if defined(__GNUC__) || defined(__clang__)

It also fixes another minor implicit conversion warning.

@chausner chausner changed the title Fix MSVC unknown pragma warning Fix two MSVC warnings Dec 15, 2021
@tstenner
Copy link
Collaborator

Looks mostly good to me; getting rid of the compiler warnings so it's easier to notice when new ones pop up is a good idea.

Two minor things:

Before this change, this pragma was active when compiling with clang. I am not sure if this was on purpose and whether it has any effect on clang.

Clang accepts most of the GCC warnings and accepts #pragma GCC (see https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas ). For the future it might make sense to define NOWARN_MSVC(), NOWARN_GCC_AND_CLANG() and NOWARN_CLANG() macros, but for now your suggestion is fine.

It also fixes another minor implicit conversion warning.

Looks good, the only thing I'd improve would be static_cast, as it's more limited in scope than C style casts.

@chausner
Copy link
Contributor Author

chausner commented Jan 6, 2022

This would be ready to merge from my side or do you want me to make any more changes?

@tstenner tstenner merged commit dbbe715 into sccn:master Feb 19, 2022
@chausner chausner deleted the fix-msvc-pragma-warning branch February 19, 2022 13:20
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.

None yet

2 participants