Commit 7b315782e1eede605325c94a280cfd1272d438d4

Authored by Henry Schreiner
Committed by GitHub
1 parent acee69a8

Warnings (#281)

* Fixing some warnings

* Make gtest a system library

* Fixing format

* Adding better method for adding warnings

* Nicer Windows deprecated test

* JSON update and drop testing timer

* Warnings as errors everywhere
.appveyor.yml
... ... @@ -13,7 +13,7 @@ install:
13 13 build_script:
14 14 - mkdir build
15 15 - cd build
16   - - ps: cmake .. -DCLI11_SINGLE_FILE_TESTS=ON -DCMAKE_BUILD_TYPE=Debug -DCMAKE_GENERATOR="Visual Studio 14 2015"
  16 + - ps: cmake .. -DCLI11_WARNINGS_AS_ERRORS=ON -DCLI11_SINGLE_FILE_TESTS=ON -DCMAKE_BUILD_TYPE=Debug -DCMAKE_GENERATOR="Visual Studio 14 2015"
17 17 - ps: cmake --build .
18 18 - cd ..
19 19 - conan create . CLIUtils/CLI11
... ...
.ci/azure-build.yml
... ... @@ -2,7 +2,7 @@ steps:
2 2  
3 3 - task: CMake@1
4 4 inputs:
5   - cmakeArgs: .. -DCLI11_SINGLE_FILE=$(cli11.single) -DCLI11_CXX_STD=$(cli11.std) -DCLI11_SINGLE_FILE_TESTS=$(cli11.single) -DCMAKE_BUILD_TYPE=$(cli11.build_type) $(cli11.options)
  5 + cmakeArgs: .. -DCLI11_WARNINGS_AS_ERRORS=ON -DCLI11_SINGLE_FILE=$(cli11.single) -DCLI11_CXX_STD=$(cli11.std) -DCLI11_SINGLE_FILE_TESTS=$(cli11.single) -DCMAKE_BUILD_TYPE=$(cli11.build_type) $(cli11.options)
6 6 displayName: 'Configure'
7 7  
8 8 - script: cmake --build .
... ...
.ci/make_and_test.sh
... ... @@ -8,7 +8,7 @@ set -evx
8 8  
9 9 mkdir -p build
10 10 cd build
11   -cmake .. -DCLI11_SINGLE_FILE=ON -DCLI11_CXX_STD=$STD -DCLI11_SINGLE_FILE_TESTS=ON -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_COMPILER_LAUNCHER=ccache $@
  11 +cmake .. -DCLI11_WARNINGS_AS_ERRORS=ON -DCLI11_SINGLE_FILE=ON -DCLI11_CXX_STD=$STD -DCLI11_SINGLE_FILE_TESTS=ON -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_COMPILER_LAUNCHER=ccache $@
12 12 cmake --build . -- -j2
13 13  
14 14 set +evx
... ...
CMakeLists.txt
... ... @@ -11,9 +11,6 @@ else()
11 11 cmake_policy(VERSION 3.14)
12 12 endif()
13 13  
14   -# TESTING: remove this later
15   -set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE "${CMAKE_COMMAND} -E time")
16   -
17 14 set(VERSION_REGEX "#define CLI11_VERSION[ \t]+\"(.+)\"")
18 15  
19 16 # Read in the line containing the version
... ... @@ -26,6 +23,9 @@ string(REGEX REPLACE ${VERSION_REGEX} "\\1" VERSION_STRING "${VERSION_STRING}")
26 23 # Add the project
27 24 project(CLI11 LANGUAGES CXX VERSION ${VERSION_STRING})
28 25  
  26 +# Special target that adds warnings. Is not exported.
  27 +add_library(CLI11_warnings INTERFACE)
  28 +
29 29 # Only if built as the main project
30 30 if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)
31 31 # User settable
... ... @@ -45,11 +45,19 @@ if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)
45 45 set(CMAKE_CXX_EXTENSIONS OFF)
46 46 set(CMAKE_CXX_STANDARD_REQUIRED ON)
47 47  
  48 + option(CLI11_WARNINGS_AS_ERRORS "Turn all warnings into errors (for CI)")
  49 +
48 50 # Be moderately paranoid with flags
49 51 if(MSVC)
50   - add_definitions("/W4")
  52 + target_compile_options(CLI11_warnings INTERFACE "/W4")
  53 + if(CLI11_WARNINGS_AS_ERRORS)
  54 + target_compile_options(CLI11_warnings INTERFACE "/WX")
  55 + endif()
51 56 else()
52   - add_definitions(-Wall -Wextra -pedantic -Wshadow)
  57 + target_compile_options(CLI11_warnings INTERFACE -Wall -Wextra -pedantic -Wshadow)
  58 + if(CLI11_WARNINGS_AS_ERRORS)
  59 + target_compile_options(CLI11_warnings INTERFACE -Werror)
  60 + endif()
53 61 endif()
54 62  
55 63 if(CMAKE_VERSION VERSION_GREATER 3.6)
... ...
cmake/AddGoogletest.cmake
1   -#
  1 +#
2 2 #
3 3 # Downloads GTest and provides a helper macro to add tests. Add make check, as well, which
4 4 # gives output on failed tests without having to set an environment variable.
... ... @@ -25,7 +25,7 @@ endif()
25 25 # Target must already exist
26 26 macro(add_gtest TESTNAME)
27 27 target_link_libraries(${TESTNAME} PUBLIC gtest gmock gtest_main)
28   -
  28 +
29 29 if(GOOGLE_TEST_INDIVIDUAL)
30 30 if(CMAKE_VERSION VERSION_LESS 3.10)
31 31 gtest_add_tests(TARGET ${TESTNAME}
... ... @@ -36,7 +36,7 @@ macro(add_gtest TESTNAME)
36 36 gtest_discover_tests(${TESTNAME}
37 37 TEST_PREFIX "${TESTNAME}."
38 38 PROPERTIES FOLDER "Tests")
39   -
  39 +
40 40 endif()
41 41 else()
42 42 add_test(${TESTNAME} ${TESTNAME})
... ... @@ -59,6 +59,13 @@ BUILD_GTEST
59 59 set_target_properties(gtest gtest_main gmock gmock_main
60 60 PROPERTIES FOLDER "Extern")
61 61  
  62 +foreach(TGT IN ITEMS gtest gtest_main gmock gmock_main)
  63 + get_property(DIR_LIST TARGET ${TGT} PROPERTY INTERFACE_INCLUDE_DIRECTORIES)
  64 + foreach(ITEM IN LISTS DIR_LIST)
  65 + set_property(TARGET ${TGT} APPEND PROPERTY INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${ITEM}")
  66 + endforeach()
  67 +endforeach()
  68 +
62 69 if(MSVC)
63 70 if (MSVC_VERSION GREATER_EQUAL 1900)
64 71 target_compile_definitions(gtest PUBLIC _SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING)
... ...
examples/CMakeLists.txt
... ... @@ -19,8 +19,8 @@ if(CLI11_EXAMPLE_JSON)
19 19 if(NOT EXISTS "${CLI11_SOURCE_DIR}/extern/json/single_include/nlohmann/json.hpp")
20 20 message(ERROR "You are missing the json package for CLI11_EXAMPLE_JSON. Please update your submodules (git submodule update --init)")
21 21 endif()
22   - if(CMAKE_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.9)
23   - message(WARNING "The json example requires GCC 4.9+ (requirement on json library)")
  22 + if(CMAKE_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.8)
  23 + message(WARNING "The json example requires GCC 4.8+ (requirement on json library)")
24 24 endif()
25 25 add_cli_exe(json json.cpp)
26 26 target_include_directories(json PUBLIC SYSTEM "${CLI11_SOURCE_DIR}/extern/json/single_include")
... ... @@ -69,7 +69,7 @@ set_property(TEST subcom_partitioned_none PROPERTY PASS_REGULAR_EXPRESSION
69 69 "This is a timer:"
70 70 "--file is required"
71 71 "Run with --help for more information.")
72   -
  72 +
73 73 add_test(NAME subcom_partitioned_all COMMAND subcom_partitioned --file this --count --count -d 1.2)
74 74 set_property(TEST subcom_partitioned_all PROPERTY PASS_REGULAR_EXPRESSION
75 75 "This is a timer:"
... ... @@ -93,7 +93,7 @@ set_property(TEST option_groups_extra PROPERTY PASS_REGULAR_EXPRESSION
93 93 add_test(NAME option_groups_extra2 COMMAND option_groups --csv --address "192.168.1.1" -o "test.out")
94 94 set_property(TEST option_groups_extra2 PROPERTY PASS_REGULAR_EXPRESSION
95 95 "at most 1")
96   -
  96 +
97 97 add_cli_exe(positional_arity positional_arity.cpp)
98 98 add_test(NAME positional_arity1 COMMAND positional_arity one )
99 99 set_property(TEST positional_arity1 PROPERTY PASS_REGULAR_EXPRESSION
... ... @@ -132,7 +132,7 @@ set_property(TEST shapes_all PROPERTY PASS_REGULAR_EXPRESSION
132 132 "circle4"
133 133 "rectangle2 with edges [2.1,2.1]"
134 134 "triangel1 with sides [4.5]")
135   -
  135 +
136 136 add_cli_exe(ranges ranges.cpp)
137 137 add_test(NAME ranges_range COMMAND ranges --range 1 2 3)
138 138 set_property(TEST ranges_range PROPERTY PASS_REGULAR_EXPRESSION
... ...
1   -Subproject commit db53bdac1926d1baebcb459b685dcd2e4608c355
  1 +Subproject commit 1126c9ca74fdea22d2ce3a065ac0fcb5792cbdaf
... ...
sanitizers @ 99e159ec9bc
1   -Subproject commit 6947cff3a9c9305eb9c16135dd81da3feb4bf87f
  1 +Subproject commit 99e159ec9bc8dd362b08d18436bd40ff0648417b
... ...
include/CLI/App.hpp
... ... @@ -1105,8 +1105,11 @@ class App {
1105 1105 }
1106 1106 /// Get a pointer to subcommand by index
1107 1107 App *get_subcommand(int index = 0) const {
1108   - if((index >= 0) && (index < static_cast<int>(subcommands_.size())))
1109   - return subcommands_[index].get();
  1108 + if(index >= 0) {
  1109 + auto uindex = static_cast<unsigned>(index);
  1110 + if(uindex < subcommands_.size())
  1111 + return subcommands_[uindex].get();
  1112 + }
1110 1113 throw OptionNotFound(std::to_string(index));
1111 1114 }
1112 1115  
... ... @@ -1130,8 +1133,11 @@ class App {
1130 1133  
1131 1134 /// Get an owning pointer to subcommand by index
1132 1135 CLI::App_p get_subcommand_ptr(int index = 0) const {
1133   - if((index >= 0) && (index < static_cast<int>(subcommands_.size())))
1134   - return subcommands_[index];
  1136 + if(index >= 0) {
  1137 + auto uindex = static_cast<unsigned>(index);
  1138 + if(uindex < subcommands_.size())
  1139 + return subcommands_[uindex];
  1140 + }
1135 1141 throw OptionNotFound(std::to_string(index));
1136 1142 }
1137 1143  
... ... @@ -1274,13 +1280,13 @@ class App {
1274 1280 /// This must be called after the options are in but before the rest of the program.
1275 1281 void parse(int argc, const char *const *argv) {
1276 1282 // If the name is not set, read from command line
1277   - if((name_.empty()) || (has_automatic_name_)) {
  1283 + if(name_.empty() || has_automatic_name_) {
1278 1284 has_automatic_name_ = true;
1279 1285 name_ = argv[0];
1280 1286 }
1281 1287  
1282 1288 std::vector<std::string> args;
1283   - args.reserve(argc - 1);
  1289 + args.reserve(static_cast<size_t>(argc - 1));
1284 1290 for(int i = argc - 1; i > 0; i--)
1285 1291 args.emplace_back(argv[i]);
1286 1292 parse(std::move(args));
... ...
include/CLI/Option.hpp
... ... @@ -722,7 +722,7 @@ class Option : public OptionBase&lt;Option&gt; {
722 722 } else if(get_items_expected() < 0) {
723 723 // Require that this be a multiple of expected size and at least as many as expected
724 724 if(results_.size() < static_cast<size_t>(-get_items_expected()) ||
725   - results_.size() % static_cast<size_t>(std::abs(get_type_size())) != 0)
  725 + results_.size() % static_cast<size_t>(std::abs(get_type_size())) != 0u)
726 726 throw ArgumentMismatch(get_name(), get_items_expected(), results_.size());
727 727 }
728 728 local_result = !callback_(results_);
... ... @@ -799,7 +799,8 @@ class Option : public OptionBase&lt;Option&gt; {
799 799 if(!((input_value.empty()) || (input_value == emptyString))) {
800 800 auto default_ind = detail::find_member(name, fnames_, ignore_case_, ignore_underscore_);
801 801 if(default_ind >= 0) {
802   - if(default_flag_values_[default_ind].second != input_value) {
  802 + // We can static cast this to size_t because it is more than 0 in this block
  803 + if(default_flag_values_[static_cast<size_t>(default_ind)].second != input_value) {
803 804 throw(ArgumentMismatch::FlagOverride(name));
804 805 }
805 806 } else {
... ... @@ -811,12 +812,12 @@ class Option : public OptionBase&lt;Option&gt; {
811 812 }
812 813 auto ind = detail::find_member(name, fnames_, ignore_case_, ignore_underscore_);
813 814 if((input_value.empty()) || (input_value == emptyString)) {
814   - return (ind < 0) ? trueString : default_flag_values_[ind].second;
  815 + return (ind < 0) ? trueString : default_flag_values_[static_cast<size_t>(ind)].second;
815 816 }
816 817 if(ind < 0) {
817 818 return input_value;
818 819 }
819   - if(default_flag_values_[ind].second == falseString) {
  820 + if(default_flag_values_[static_cast<size_t>(ind)].second == falseString) {
820 821 try {
821 822 auto val = detail::to_flag_value(input_value);
822 823 return (val == 1) ? falseString : (val == (-1) ? trueString : std::to_string(-val));
... ...
include/CLI/StringTools.hpp
... ... @@ -231,11 +231,12 @@ inline bool has_default_flag_values(const std::string &amp;flags) {
231 231 }
232 232  
233 233 inline void remove_default_flag_values(std::string &flags) {
234   - size_t loc = flags.find_first_of('{');
  234 + auto loc = flags.find_first_of('{');
235 235 while(loc != std::string::npos) {
236 236 auto finish = flags.find_first_of("},", loc + 1);
237 237 if((finish != std::string::npos) && (flags[finish] == '}')) {
238   - flags.erase(flags.begin() + loc, flags.begin() + finish + 1);
  238 + flags.erase(flags.begin() + static_cast<std::ptrdiff_t>(loc),
  239 + flags.begin() + static_cast<std::ptrdiff_t>(finish) + 1);
239 240 }
240 241 loc = flags.find_first_of('{', loc + 1);
241 242 }
... ...
include/CLI/Timer.hpp
... ... @@ -68,9 +68,9 @@ class Timer {
68 68 f();
69 69 std::chrono::duration<double> elapsed = clock::now() - start_;
70 70 total_time = elapsed.count();
71   - } while(n++ < 100 && total_time < target_time);
  71 + } while(n++ < 100u && total_time < target_time);
72 72  
73   - std::string out = make_time_str(total_time / n) + " for " + std::to_string(n) + " tries";
  73 + std::string out = make_time_str(total_time / static_cast<double>(n)) + " for " + std::to_string(n) + " tries";
74 74 start_ = start;
75 75 return out;
76 76 }
... ... @@ -79,7 +79,7 @@ class Timer {
79 79 std::string make_time_str() const {
80 80 time_point stop = clock::now();
81 81 std::chrono::duration<double> elapsed = stop - start_;
82   - double time = elapsed.count() / cycles;
  82 + double time = elapsed.count() / static_cast<double>(cycles);
83 83 return make_time_str(time);
84 84 }
85 85  
... ...
include/CLI/Validators.hpp
... ... @@ -800,7 +800,7 @@ class AsNumberWithUnit : public Validator {
800 800 }
801 801  
802 802 std::string unit{unit_begin, input.end()};
803   - input.resize(std::distance(input.begin(), unit_begin));
  803 + input.resize(static_cast<size_t>(std::distance(input.begin(), unit_begin)));
804 804 detail::trim(input);
805 805  
806 806 if(opts & UNIT_REQUIRED && unit.empty()) {
... ...
tests/CMakeLists.txt
... ... @@ -55,7 +55,7 @@ foreach(T ${CLI11_TESTS})
55 55  
56 56 add_executable(${T} ${T}.cpp ${CLI11_headers})
57 57 add_sanitizers(${T})
58   - target_link_libraries(${T} PUBLIC CLI11)
  58 + target_link_libraries(${T} PUBLIC CLI11 CLI11_warnings)
59 59 add_gtest(${T})
60 60  
61 61 if(CLI11_SINGLE_FILE AND CLI11_SINGLE_FILE_TESTS)
... ... @@ -84,6 +84,11 @@ if(NOT MSVC)
84 84 if(TARGET DeprecatedTest_Single)
85 85 target_compile_options(DeprecatedTest_Single PRIVATE -Wno-deprecated-declarations)
86 86 endif()
  87 +else()
  88 + target_compile_options(DeprecatedTest PRIVATE "/wd4996")
  89 + if(TARGET DeprecatedTest_Single)
  90 + target_compile_options(DeprecatedTest_Single PRIVATE "/wd4996")
  91 + endif()
87 92 endif()
88 93  
89 94 # Link test (build error if inlines missing)
... ...
tests/HelpersTest.cpp
... ... @@ -86,7 +86,7 @@ TEST(StringTools, Modify3) {
86 86 std::string newString = CLI::detail::find_and_modify("baaaaaaaaaa", "aaa", [](std::string &str, size_t index) {
87 87 str.erase(index, 3);
88 88 str.insert(str.begin(), 'a');
89   - return 0;
  89 + return 0u;
90 90 });
91 91 EXPECT_EQ(newString, "aba");
92 92 }
... ...
tests/TransformTest.cpp
... ... @@ -612,7 +612,7 @@ TEST_F(TApp, NumberWithUnitIntOverflow) {
612 612 }
613 613  
614 614 TEST_F(TApp, NumberWithUnitFloatOverflow) {
615   - std::map<std::string, float> mapping{{"a", 2}, {"b", 1}, {"c", 0}};
  615 + std::map<std::string, float> mapping{{"a", 2.f}, {"b", 1.f}, {"c", 0.f}};
616 616  
617 617 float value;
618 618 app.add_option("-n", value)->transform(CLI::AsNumberWithUnit(mapping));
... ... @@ -622,11 +622,11 @@ TEST_F(TApp, NumberWithUnitFloatOverflow) {
622 622  
623 623 args = {"-n", "3e+38 b"};
624 624 run();
625   - EXPECT_FLOAT_EQ(value, 3e+38);
  625 + EXPECT_FLOAT_EQ(value, 3e+38f);
626 626  
627 627 args = {"-n", "3e+38 c"};
628 628 run();
629   - EXPECT_FLOAT_EQ(value, 0);
  629 + EXPECT_FLOAT_EQ(value, 0.f);
630 630 }
631 631  
632 632 TEST_F(TApp, AsSizeValue1000_1024) {
... ... @@ -639,7 +639,7 @@ TEST_F(TApp, AsSizeValue1000_1024) {
639 639  
640 640 args = {"-s", "1b"};
641 641 run();
642   - EXPECT_FLOAT_EQ(value, 1);
  642 + EXPECT_EQ(value, 1u);
643 643  
644 644 uint64_t k_value = 1000u;
645 645 uint64_t ki_value = 1024u;
... ... @@ -745,7 +745,7 @@ TEST_F(TApp, AsSizeValue1024) {
745 745  
746 746 args = {"-s", "1b"};
747 747 run();
748   - EXPECT_FLOAT_EQ(value, 1);
  748 + EXPECT_EQ(value, 1u);
749 749  
750 750 uint64_t ki_value = 1024u;
751 751 args = {"-s", "1k"};
... ...