Add warnings options #90
Conversation
# Conflicts: # include/sparrow/c_interface.hpp
Alex-PLACET
force-pushed
the
warning_flags
branch
from
807f903
to
530cffe
Compare
| # /w14928 # illegal copy-initialization; more than one user-defined conversion has been implicitly applied | ||
| # /we4289 # nonstandard extension used: 'variable': loop control variable declared in the for-loop is used outside the for-loop scope | ||
| # /we4244 # conversion from 'type1' to 'type_2', possible loss of data | ||
| /Wall # all warnings |
There was a problem hiding this comment.
This one is not recommended (see https://github.com/cpp-best-practices/cppbestpractices/blob/master/02-Use_the_Tools_Available.md#msvc ).
I suspect removing this will make the rest work better.
There was a problem hiding this comment.
As I said in this comment https://github.com/xtensor-stack/sparrow/pull/90/files/166a1fd555ec981620e3b152c0e13872a3183be2#r1609579182, we have two choices, Wall and deactivate the problematic ones or activate warnings one by one
There was a problem hiding this comment.
No because /Wall activates warning from dependencies (in addition to warning-as-error), which is what Turner points and dont recommend. It's not something we can easilly turn off without dependency-specific flags. That's why it's not recommended with msvc. It's not like gcc/clang's -Wall which does not activate all warnings, but only recomended ones.
There was a problem hiding this comment.
It's the point of using /external:anglebrackets /external:W0 which deactivate warnings from the std and third parties (everything included with angle brackets)
There was a problem hiding this comment.
It is not enough to disable the dependencies included through " :/
Implement #88