Skip to content
This repository has been archived by the owner on Jan 3, 2023. It is now read-only.

Commit

Permalink
Cyphers/lastwarns (#3640)
Browse files Browse the repository at this point in the history 
* Build changes for -Wall

* Use properties

* Remove no-zero-as-null-pointer-constant

* add ##__VAR_ARGS__ workaround

* enable gnu-zero-variadic-macro-arguments warning

* fix gnu-zero-variadic-macro-arguments

* use PrintToDummyParamName

* remove ##__VA_ARGS__ workaround

* fix ##__VA_ARGS__ and enable gnu-zero-variadic-macro-arguments

* handle windows build

* use alternative fix to support VS compiler
  • Loading branch information
@diyessi
diyessi authored Sep 25, 2019
1 parent 68472cc commit 1cc365c
Show file tree
Hide file tree
Showing 12 changed files with 127 additions and 36 deletions.
2 changes: 2 additions & 0 deletions CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,8 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
endif()
endif()

set(CMAKE_ORIGINAL_CXX_FLAGS "${CMAKE_CXX_FLAGS}")

ngraph_var(NGRAPH_WARNINGS_AS_ERRORS DEFAULT "OFF")
if (${NGRAPH_WARNINGS_AS_ERRORS})
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
Expand Down
28 changes: 12 additions & 16 deletions cmake/clang_4_0_flags.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,19 @@ add_compile_options(-Werror=comment)
add_compile_options(-pedantic-errors)

# whitelist errors here
add_compile_options(-Weverything)
add_compile_options(-Wno-gnu-zero-variadic-macro-arguments)
add_compile_options(-Wno-c++98-compat-pedantic)
add_compile_options(-Wno-weak-vtables)
add_compile_options(-Wno-global-constructors)
add_compile_options(-Wno-exit-time-destructors)
add_compile_options(-Wno-missing-prototypes)
add_compile_options(-Wno-missing-noreturn)
add_compile_options(-Wno-covered-switch-default)
add_compile_options(-Wno-undef)
#add_compile_options(-Weverything)
add_compile_options(-Wall)
# add_compile_options(-Wno-gnu-zero-variadic-macro-arguments)
#add_compile_options(-Wno-c++98-compat-pedantic)
#add_compile_options(-Wno-weak-vtables)
#add_compile_options(-Wno-global-constructors)
#add_compile_options(-Wno-exit-time-destructors)
#add_compile_options(-Wno-missing-prototypes)
#add_compile_options(-Wno-missing-noreturn)
#add_compile_options(-Wno-covered-switch-default)
#add_compile_options(-Wno-undef)
if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "AppleClang")
if (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 9.1.0)
add_compile_options(-Wno-zero-as-null-pointer-constant)
#add_compile_options(-Wno-zero-as-null-pointer-constant)
endif()
endif()

# should remove these
add_compile_options(-Wno-padded)
add_compile_options(-Wno-conversion)
add_compile_options(-Wno-double-promotion)
6 changes: 3 additions & 3 deletions cmake/external_onnx.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ ExternalProject_Add(
CMAKE_GENERATOR_PLATFORM ${CMAKE_GENERATOR_PLATFORM}
CMAKE_GENERATOR_TOOLSET ${CMAKE_GENERATOR_TOOLSET}
CMAKE_ARGS ${NGRAPH_FORWARD_CMAKE_ARGS}
-DCMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}
-DCMAKE_CXX_FLAGS=${CMAKE_ORIGINAL_CXX_FLAGS}
-DONNX_GEN_PB_TYPE_STUBS=OFF
-DCMAKE_PREFIX_PATH=${Protobuf_INSTALL_PREFIX}
-DONNX_ML=TRUE
Expand Down Expand Up @@ -85,15 +85,15 @@ set(ONNX_LIBRARIES ${ONNX_LIBRARY} ${ONNX_PROTO_LIBRARY})
if (NOT TARGET onnx::libonnx)
add_library(onnx::libonnx UNKNOWN IMPORTED)
set_target_properties(onnx::libonnx PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES ${ONNX_INCLUDE_DIR}
INTERFACE_SYSTEM_INCLUDE_DIRECTORIES ${ONNX_INCLUDE_DIR}
IMPORTED_LOCATION ${ONNX_LIBRARY})
add_dependencies(onnx::libonnx ext_onnx)
endif()

if (NOT TARGET onnx::libonnx_proto)
add_library(onnx::libonnx_proto UNKNOWN IMPORTED)
set_target_properties(onnx::libonnx_proto PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES ${ONNX_PROTO_INCLUDE_DIR}
INTERFACE_SYSTEM_INCLUDE_DIRECTORIES ${ONNX_PROTO_INCLUDE_DIR}
IMPORTED_LOCATION ${ONNX_PROTO_LIBRARY})
add_dependencies(onnx::libonnx_proto ext_onnx)
endif()
5 changes: 3 additions & 2 deletions cmake/external_protobuf.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ if (WIN32)
CMAKE_GENERATOR_TOOLSET ${CMAKE_GENERATOR_TOOLSET}
CMAKE_ARGS
${NGRAPH_FORWARD_CMAKE_ARGS}
-DCMAKE_CXX_FLAGS=${CMAKE_CXX_FLAGS}
-DCMAKE_CXX_FLAGS=${CMAKE_ORIGINAL_CXX_FLAGS}
-Dprotobuf_MSVC_STATIC_RUNTIME=OFF
-Dprotobuf_WITH_ZLIB=OFF
-Dprotobuf_BUILD_TESTS=OFF
Expand Down Expand Up @@ -108,14 +108,15 @@ set(Protobuf_LIBRARIES ${Protobuf_LIBRARY})
if (NOT TARGET protobuf::libprotobuf)
add_library(protobuf::libprotobuf UNKNOWN IMPORTED)
set_target_properties(protobuf::libprotobuf PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES "${Protobuf_INCLUDE_DIR}"
INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${Protobuf_INCLUDE_DIR}"
IMPORTED_LOCATION "${Protobuf_LIBRARY}")
add_dependencies(protobuf::libprotobuf ext_protobuf)
endif()

if (NOT TARGET protobuf::protoc)
add_executable(protobuf::protoc IMPORTED)
set_target_properties(protobuf::protoc PROPERTIES
INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${Protobuf_PROTOC_EXECUTABLE}"
IMPORTED_LOCATION "${Protobuf_PROTOC_EXECUTABLE}")
add_dependencies(protobuf::protoc ext_protobuf)
endif()
Expand Down
87 changes: 82 additions & 5 deletions src/ngraph/check.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,29 +136,106 @@ namespace ngraph
// TODO(amprocte): refactor NGRAPH_CHECK_HELPER so we don't have to introduce a locally-scoped
// variable (ss___) and risk shadowing.
//
#define NGRAPH_CHECK_HELPER(exc_class, ctx, check, ...) \
#define NGRAPH_CHECK_HELPER2(exc_class, ctx, check, ...) \
do \
{ \
if (!(check)) \
{ \
::std::stringstream ss___; \
::ngraph::write_all_to_stream(ss___, ##__VA_ARGS__); \
::ngraph::write_all_to_stream(ss___, __VA_ARGS__); \
throw exc_class( \
(::ngraph::CheckLocInfo{__FILE__, __LINE__, #check}), (ctx), ss___.str()); \
} \
} while (0)

#define NGRAPH_CHECK_HELPER1(exc_class, ctx, check) \
do \
{ \
if (!(check)) \
{ \
throw exc_class((::ngraph::CheckLocInfo{__FILE__, __LINE__, #check}), (ctx), ""); \
} \
} while (0)

/// \brief Macro to check whether a boolean condition holds.
/// \param cond Condition to check
/// \param ... Additional error message info to be added to the error message via the `<<`
/// stream-insertion operator. Note that the expressions here will be evaluated lazily,
/// i.e., only if the `cond` evalutes to `false`.
/// \throws ::ngraph::CheckFailure if `cond` is false.
#define NGRAPH_CHECK(cond, ...) \
NGRAPH_CHECK_HELPER(::ngraph::CheckFailure, "", (cond), ##__VA_ARGS__)
#define NGRAPH_CHECK(...) NGRAPH_CHECK_HELPER(::ngraph::CheckFailure, "", __VA_ARGS__)

/// \brief Macro to signal a code path that is unreachable in a successful execution. It's
/// implemented with NGRAPH_CHECK macro.
/// \param ... Additional error message that should describe why that execution path is unreachable.
/// \throws ::ngraph::CheckFailure if the macro is executed.
#define NGRAPH_UNREACHABLE(...) NGRAPH_CHECK(false, "Unreachable: ", ##__VA_ARGS__)
#define NGRAPH_UNREACHABLE(...) NGRAPH_CHECK(false, "Unreachable: ", __VA_ARGS__)
#define NGRAPH_CHECK_HELPER(exc_class, ctx, ...) \
CALL_OVERLOAD(NGRAPH_CHECK_HELPER, exc_class, ctx, __VA_ARGS__)

#define GLUE(x, y) x y

#define RETURN_ARG_COUNT(_1_, \
_2_, \
_3_, \
_4_, \
_5_, \
_6, \
_7, \
_8, \
_9, \
_10, \
_11, \
_12, \
_13, \
_14, \
_15, \
_16, \
_17, \
_18, \
_19, \
_20, \
_21, \
_22, \
_23, \
_24, \
_25, \
count, \
...) \
count
#define EXPAND_ARGS(args) RETURN_ARG_COUNT args
#define COUNT_ARGS_MAXN(...) \
EXPAND_ARGS((__VA_ARGS__, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
2, \
1, \
0))

#define OVERLOAD_MACRO2(name, count) name##count
#define OVERLOAD_MACRO1(name, count) OVERLOAD_MACRO2(name, count)
#define OVERLOAD_MACRO(name, count) OVERLOAD_MACRO1(name, count)

#define CALL_OVERLOAD(name, exc_class, ctx, ...) \
GLUE(OVERLOAD_MACRO(name, COUNT_ARGS_MAXN(__VA_ARGS__)), (exc_class, ctx, __VA_ARGS__))
5 changes: 3 additions & 2 deletions src/ngraph/frontend/onnx_import/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,15 @@ add_dependencies(onnx_import_interface protobuf::libprotobuf onnx::libonnx onnx:
add_dependencies(onnx_import onnx_import_interface)

set_property(TARGET onnx_import PROPERTY POSITION_INDEPENDENT_CODE ON)
target_include_directories(onnx_import PRIVATE ${ONNX_IMPORT_INCLUDE_DIR} ${NGRAPH_INCLUDE_PATH}
target_include_directories(onnx_import
SYSTEM PRIVATE ${ONNX_IMPORT_INCLUDE_DIR} ${NGRAPH_INCLUDE_PATH}
SYSTEM PRIVATE ${ONNX_INCLUDE_DIR} ${ONNX_PROTO_INCLUDE_DIR} ${Protobuf_INCLUDE_DIR})
target_link_libraries(onnx_import PRIVATE ${Protobuf_LIBRARIES} ${ONNX_PROTO_LIBRARY})

target_compile_definitions(onnx_import PRIVATE ONNX_OPSET_VERSION=${ONNX_OPSET_VERSION})

set_property(TARGET onnx_import_interface PROPERTY POSITION_INDEPENDENT_CODE ON)
target_include_directories(onnx_import_interface PRIVATE ${ONNX_IMPORT_INCLUDE_DIR} ${NGRAPH_INCLUDE_PATH}
target_include_directories(onnx_import_interface SYSTEM PRIVATE ${ONNX_IMPORT_INCLUDE_DIR} ${NGRAPH_INCLUDE_PATH}
SYSTEM PRIVATE ${ONNX_INCLUDE_DIR} ${ONNX_PROTO_INCLUDE_DIR} ${Protobuf_INCLUDE_DIR})

target_compile_definitions(onnx_import_interface PRIVATE ONNX_OPSET_VERSION=${ONNX_OPSET_VERSION})
Expand Down
4 changes: 2 additions & 2 deletions src/ngraph/node.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -782,8 +782,8 @@ namespace ngraph
bool m_is_short;
};
}
#define NODE_VALIDATION_CHECK(node, cond, ...) \
NGRAPH_CHECK_HELPER(::ngraph::NodeValidationFailure, (node), (cond), ##__VA_ARGS__)
#define NODE_VALIDATION_CHECK(node, ...) \
NGRAPH_CHECK_HELPER(::ngraph::NodeValidationFailure, (node), __VA_ARGS__)

namespace ngraph
{
Expand Down
2 changes: 1 addition & 1 deletion test/check.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ TEST(check, check_with_explanation)
catch (const CheckFailure& e)
{
check_failure_thrown = true;
EXPECT_PRED_FORMAT2(testing::IsSubstring, "Check '(false)' failed at", e.what());
EXPECT_PRED_FORMAT2(testing::IsSubstring, "Check 'false' failed at", e.what());
EXPECT_PRED_FORMAT2(testing::IsSubstring, "xyzzyxyzzy123", e.what());
}

Expand Down
3 changes: 2 additions & 1 deletion test/type_prop/dyn_replace_slice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,8 @@ INSTANTIATE_TEST_CASE_P(
{1},
{}},
DynReplaceSliceParams{
{1, 2, 3}, {3}, {3}, {0}, {1, 1, 2}, {0, 0, 1}, {2, 2, 2}, {}, {}, {}, {0}, {2}, {1}}));
{1, 2, 3}, {3}, {3}, {0}, {1, 1, 2}, {0, 0, 1}, {2, 2, 2}, {}, {}, {}, {0}, {2}, {1}}),
PrintToDummyParamName());

void DynReplaceSlice_Test_Shape_Except(const shared_ptr<Node>& param_0,
const shared_ptr<Node>& param_1,
Expand Down
3 changes: 2 additions & 1 deletion test/type_prop/dyn_slice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ INSTANTIATE_TEST_CASE_P(
{{1}, {1}, {}, {1}, {}}),
DynSliceParams({{1, 2, 3}, {3}, {3}, {0}, {1, 1, 2}},
{{0, 0, 1}, {2, 2, 2}, {}},
{{}, {}, {0}, {2}, {1}})));
{{}, {}, {0}, {2}, {1}})),
PrintToDummyParamName());

void DynSlice_Test_Shape_Except(const shared_ptr<Node>& param_0,
const shared_ptr<Node>& param_1,
Expand Down
9 changes: 6 additions & 3 deletions test/type_prop/range.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,8 @@ INSTANTIATE_TEST_CASE_P(type_prop,
RangeParams{1, 23, 2, PartialShape{11}},
RangeParams{1, 22, 2, PartialShape{11}},
RangeParams{0, 0, 1, PartialShape{0}},
RangeParams{1, 0, 2, PartialShape{0}}));
RangeParams{1, 0, 2, PartialShape{0}}),
PrintToDummyParamName());

struct RangeTestWithNegatives : ::testing::TestWithParam<RangeParams>
{
Expand Down Expand Up @@ -488,7 +489,8 @@ INSTANTIATE_TEST_CASE_P(type_prop,
RangeParams{2, 0, -1, PartialShape{2}},
RangeParams{-19, 19, 1, PartialShape{38}},
RangeParams{-19, 19, 3, PartialShape{13}},
RangeParams{20, -19, 1, PartialShape{0}}));
RangeParams{20, -19, 1, PartialShape{0}}),
PrintToDummyParamName());

struct RangeTestFloating : ::testing::TestWithParam<RangeParams>
{
Expand Down Expand Up @@ -518,4 +520,5 @@ INSTANTIATE_TEST_CASE_P(type_prop,
RangeTestFloating,
::testing::Values(RangeParams{0, 1, 0.25, PartialShape{4}},
RangeParams{-1, 1, 0.25, PartialShape{8}},
RangeParams{-1, 0.875, 0.25, PartialShape{8}}));
RangeParams{-1, 0.875, 0.25, PartialShape{8}}),
PrintToDummyParamName());
9 changes: 9 additions & 0 deletions test/util/type_prop.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,12 @@

#define EXPECT_HAS_SUBSTRING(haystack, needle) \
EXPECT_PRED_FORMAT2(testing::IsSubstring, needle, haystack)

struct PrintToDummyParamName
{
template <class ParamType>
std::string operator()(const ::testing::TestParamInfo<ParamType>& info) const
{
return "dummy" + std::to_string(info.index);
}
};

0 comments on commit 1cc365c

Please sign in to comment.