Compiler flags cleanup #51
Conversation
add security hardening compilation flags
sfraczek
force-pushed
the
compiler-flags-cleanup
branch
from
a35d25b
to
93eb2c2
Compare
also fix clang warning: no out-of-line virtual method definitions by moving destructor definition to .cpp file.
sfraczek
force-pushed
the
compiler-flags-cleanup
branch
from
93eb2c2
to
816e5a0
Compare
src/etl_localization_rcnn.cpp
Outdated
| @@ -110,7 +110,7 @@ shared_ptr<localization::rcnn::decoded> | |||
| } | |||
|
|
|||
| cv::Size im_size{mp->width(), mp->height()}; | |||
| auto crop = settings->cropbox; | |||
| //auto crop = settings->cropbox; | |||
There was a problem hiding this comment.
Why have you left the commented variable ?
There was a problem hiding this comment.
Well, I thought maybe it's a mistake that it's not used. Maybe it should be used but for some reason it's not. We don't have tests enabled for it. I left it as a hint to someone who might work on it in the future.
Should I remove it? Someone can later look at file history if they want to look for clues what's wrong.
There was a problem hiding this comment.
You can keep this line but I suggest to add comment with explanation.
| @@ -182,6 +182,9 @@ void test_multinode_manifest(bool shuffle) | |||
| auto block = manifest.next(); | |||
| auto block_node1 = manifest_node1.next(); | |||
| auto block_node2 = manifest_node2.next(); | |||
| (void) block; | |||
There was a problem hiding this comment.
Why do we need these lines ?
There was a problem hiding this comment.
This is to prevent warning that block variable is never used. I could add volalite to also prevent optimization of this just in case. I don't know this test well. Probably @AdrShv knows more.
CMakeLists.txt
Outdated
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") | ||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror=return-type") | ||
| # should add this | ||
| #set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror=suggest-override") |
There was a problem hiding this comment.
Could we remove commented lines ?
Copied some CMake code from https://github.com/NervanaSystems/ngraph-paddle/blob/2d98502006af6c07586ee1dd2903ae6e7c2b1584/cmake/external/ngraph.cmake