fix the remaining warnings in the codebase #123
Conversation
|
managed to unearth some more warnings, fixing those first |
|
this should cover all of them. all tests pass. no warnings are shown anymore |
| y_.resize(static_cast<size_t>(size_)); | ||
| yp_.resize(static_cast<size_t>(size_)); | ||
| f_.resize(static_cast<size_t>(size_)); |
There was a problem hiding this comment.
Let's make it a little less cluttered. Consider something like this here and in similar places throughout the code.
| y_.resize(static_cast<size_t>(size_)); | |
| yp_.resize(static_cast<size_t>(size_)); | |
| f_.resize(static_cast<size_t>(size_)); | |
| auto size = static_cast<size_t>(size_); | |
| y_.resize(size); | |
| yp_.resize(size); | |
| f_.resize(size); |
| @@ -37,7 +37,7 @@ int main(int /* argc */, char const** /* argv */) | |||
| // input | |||
| induct->setExternalConnectionNodes(0, 1); | |||
| // output | |||
| induct->setExternalConnectionNodes(1, -1); | |||
| induct->setExternalConnectionNodes(1, static_cast<size_t>(-1)); | |||
There was a problem hiding this comment.
Is this conversion meaningful? This will store the largest integer, right? It works because the same casting is done for the variable this is compared to, e.g.,
but I don't think it is the intended conceptual purpose of using -1.
@reid-g should chime in here.
There was a problem hiding this comment.
it should just store the largest integer size_t can hold. i went with this as i wasn't totally sure how invasive we should be here (same applies to the other comments). i would much rather
- either set it to the maximum
size_t - or adjust the api to provide another way to communicate the same intent (
std::optionalor something)
There was a problem hiding this comment.
I think this is okay as is. The index -1 stands for the ground node. It only needs to be a very distinctive integer.
CC @reid-g
|
moved to #125 |
this pull request fixes the remaining warnings resulting from the addition of
-Wall -Wextra -Wconversion -Wpedanticto the default cxxflags. these were missed in the initial pull request due to the source files they're in being gated behind theGRIDKIT_ENABLE_IPOPTdefine.this pull request also removes some unnecessary commented out flags in the root
CMakeLists.txtfile due to them being superseded by the default flag list and also being covered by theRelWithDebInfoandDebugCMAKE_BUILD_TYPEvalues (iirc).