search

sccn / liblsl

C++ lsl library for multi-modal time-synched data transmission over the local network
Other
107 stars 63 forks source link

Fix two MSVC warnings #160

Closed chausner closed 2 years ago

chausner commented 2 years ago

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.

tstenner commented 2 years ago

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 commented 2 years ago

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