Address sign promotion warnings, add todo test

lefticus · lefticus · commit d4f02b5e67c5 · 2016-03-04T07:58:21.000-07:00
diff --git a/.decent_ci-Linux.yaml b/.decent_ci-Linux.yaml
@@ -38,4 +38,5 @@ compilers:
   - name: custom_check
     commands:
        - ./contrib/check_for_tabs.rb
+       - ./contrib/check_for_todos.rb
 
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -178,7 +178,7 @@ else()
   add_definitions(-Wall -Wextra -Wconversion -Wshadow -Wnon-virtual-dtor -Wold-style-cast -Wcast-align -Wcast-qual -Wunused -Woverloaded-virtual  -pedantic ${CPP11_FLAG})
 
   if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
-    add_definitions(-Weverything -Wno-c++98-compat-pedantic  -Wno-c++98-compat -Wno-documentation -Wno-switch-enum -Wno-weak-vtables -Wno-sign-conversion -Wno-missing-prototypes -Wno-padded -Wno-missing-noreturn -Wno-exit-time-destructors -Wno-documentation-unknown-command)
+    add_definitions(-Weverything -Wno-c++98-compat-pedantic  -Wno-c++98-compat -Wno-documentation -Wno-switch-enum -Wno-weak-vtables -Wno-missing-prototypes -Wno-padded -Wno-missing-noreturn -Wno-exit-time-destructors -Wno-documentation-unknown-command)
   else()
     add_definitions(-Wnoexcept)
   endif()
diff --git a/contrib/check_for_todos.rb b/contrib/check_for_todos.rb
@@ -0,0 +1,11 @@
+#!/usr/bin/env ruby
+
+require 'json'
+
+`grep -rPIHni 'todo' src/* include/* samples/*`.lines { |line|
+  if /(?<filename>.+(hpp|cpp|chai)):(?<linenumber>[0-9]+):(?<restofline>.+)/ =~ line 
+    puts(JSON.dump({:line => linenumber, :filename => filename, :tool => "todo_checker", :message => "todo: #{restofline.strip}", :messagetype => "info"}))
+  end
+}
+
+
diff --git a/include/chaiscript/chaiscript_defines.hpp b/include/chaiscript/chaiscript_defines.hpp
@@ -113,6 +113,12 @@ namespace chaiscript {
 #endif
   }
 
+  template<typename Iter, typename Distance>
+    Iter advance_copy(Iter iter, Distance distance) {
+      std::advance(iter, static_cast<typename std::iterator_traits<Iter>::difference_type>(distance));
+      return iter;
+    }
+
 }
 #endif
 
diff --git a/include/chaiscript/dispatchkit/bootstrap_stl.hpp b/include/chaiscript/dispatchkit/bootstrap_stl.hpp
@@ -248,13 +248,17 @@ namespace chaiscript
           m->add(
               fun(
                 [](ContainerType &c, int index) -> typename ContainerType::reference {
-                  return c.at(index);
+                  /// \todo we are prefering to keep the key as 'int' to avoid runtime conversions
+                  /// during dispatch. reevaluate
+                  return c.at(static_cast<typename ContainerType::size_type>(index));
                 }), "[]");
 
           m->add(
               fun(
                 [](const ContainerType &c, int index) -> typename ContainerType::const_reference {
-                  return c.at(index);
+                  /// \todo we are prefering to keep the key as 'int' to avoid runtime conversions
+                  /// during dispatch. reevaluate
+                  return c.at(static_cast<typename ContainerType::size_type>(index));
                 }), "[]");
 
           return m;
diff --git a/include/chaiscript/dispatchkit/dispatchkit.hpp b/include/chaiscript/dispatchkit/dispatchkit.hpp
@@ -1445,7 +1445,7 @@ namespace chaiscript
         static typename Container::const_iterator find_keyed_value(const Container &t_c, const Key &t_key, const size_t t_hint)
           {
             if (t_c.size() > t_hint && t_c[t_hint].first == t_key) {
-              return t_c.begin() + t_hint;
+              return advance_copy(t_c.begin(), t_hint);
             } else {
               return find_keyed_value(t_c, t_key);
             }
diff --git a/include/chaiscript/dispatchkit/type_info.hpp b/include/chaiscript/dispatchkit/type_info.hpp
@@ -32,11 +32,11 @@ namespace chaiscript
       CHAISCRIPT_CONSTEXPR Type_Info(bool t_is_const, bool t_is_reference, bool t_is_pointer, bool t_is_void, 
           bool t_is_arithmetic, const std::type_info *t_ti, const std::type_info *t_bare_ti)
         : m_type_info(t_ti), m_bare_type_info(t_bare_ti),
-          m_flags((t_is_const << is_const_flag)
-                + (t_is_reference << is_reference_flag)
-                + (t_is_pointer << is_pointer_flag)
-                + (t_is_void << is_void_flag)
-                + (t_is_arithmetic << is_arithmetic_flag))
+          m_flags((static_cast<unsigned int>(t_is_const) << is_const_flag)
+                + (static_cast<unsigned int>(t_is_reference) << is_reference_flag)
+                + (static_cast<unsigned int>(t_is_pointer) << is_pointer_flag)
+                + (static_cast<unsigned int>(t_is_void) << is_void_flag)
+                + (static_cast<unsigned int>(t_is_arithmetic) << is_arithmetic_flag))
       {
       }
 
diff --git a/include/chaiscript/language/chaiscript_engine.hpp b/include/chaiscript/language/chaiscript_engine.hpp
@@ -505,10 +505,10 @@ namespace chaiscript
 
         // Let's see if this is a link that we should expand
         std::vector<char> buf(2048);
-        const size_t pathlen = readlink(dllpath.c_str(), &buf.front(), buf.size());
-        if (pathlen > 0 && pathlen < buf.size())
+        const auto pathlen = readlink(dllpath.c_str(), &buf.front(), buf.size());
+        if (pathlen > 0 && static_cast<size_t>(pathlen) < buf.size())
         {
-          dllpath = std::string(&buf.front(), pathlen);
+          dllpath = std::string(&buf.front(), static_cast<size_t>(pathlen));
         }
 
         m_module_paths.insert(m_module_paths.begin(), dllpath+"/");
diff --git a/include/chaiscript/language/chaiscript_parser.hpp b/include/chaiscript/language/chaiscript_parser.hpp
@@ -147,7 +147,7 @@ namespace chaiscript
         }
 
         size_t remaining() const {
-          return std::distance(m_pos, m_end);
+          return static_cast<size_t>(std::distance(m_pos, m_end));
         }
 
         char operator*() const {
@@ -2213,8 +2213,8 @@ namespace chaiscript
 
                 switch (m_operators[t_precedence]) {
                   case(AST_Node_Type::Ternary_Cond) :
-                    m_match_stack.erase(m_match_stack.begin() + m_match_stack.size() - 2,
-                        m_match_stack.begin() + m_match_stack.size() - 1);
+                    m_match_stack.erase(advance_copy(m_match_stack.begin(), m_match_stack.size() - 2),
+                                        advance_copy(m_match_stack.begin(), m_match_stack.size() - 1));
                     if (Symbol(":")) {
                       if (!Operator(t_precedence+1)) {
                         throw exception::eval_error("Incomplete "
@@ -2239,7 +2239,8 @@ namespace chaiscript
                   case(AST_Node_Type::Bitwise_Or) :
                   case(AST_Node_Type::Comparison) :
                     assert(m_match_stack.size() > 1);
-                    m_match_stack.erase(m_match_stack.begin() + m_match_stack.size() - 2, m_match_stack.begin() + m_match_stack.size() - 1);
+                    m_match_stack.erase(advance_copy(m_match_stack.begin(), m_match_stack.size() - 2), 
+                                        advance_copy(m_match_stack.begin(), m_match_stack.size() - 1));
                     build_match<eval::Binary_Operator_AST_Node>(prev_stack_top, oper->text);
                     break;
 
diff --git a/src/main.cpp b/src/main.cpp
@@ -88,24 +88,24 @@ std::vector<std::string> default_search_paths()
   std::vector<char> buf(2048);
   ssize_t size = -1;
 
-  if ((size = readlink("/proc/self/exe", &buf.front(), buf.size())) != -1)
+  if ((size = readlink("/proc/self/exe", &buf.front(), buf.size())) >= 0)
   {
-    exepath = std::string(&buf.front(), size);
+    exepath = std::string(&buf.front(), static_cast<size_t>(size));
   }
 
   if (exepath.empty())
   {
-    if ((size = readlink("/proc/curproc/file", &buf.front(), buf.size())) != -1)
+    if ((size = readlink("/proc/curproc/file", &buf.front(), buf.size())) >= 0)
     {
-      exepath = std::string(&buf.front(), size);
+      exepath = std::string(&buf.front(), static_cast<size_t>(size));
     }
   }
 
   if (exepath.empty())
   {
-    if ((size = readlink("/proc/self/path/a.out", &buf.front(), buf.size())) != -1)
+    if ((size = readlink("/proc/self/path/a.out", &buf.front(), buf.size())) >= 0)
     {
-      exepath = std::string(&buf.front(), size);
+      exepath = std::string(&buf.front(), static_cast<size_t>(size));
     }
   }
 
diff --git a/unittests/multithreaded_test.cpp b/unittests/multithreaded_test.cpp
@@ -15,7 +15,7 @@ int expected_value(int num_iters)
   return i;
 }
 
-void do_work(chaiscript::ChaiScript &c, int id)
+void do_work(chaiscript::ChaiScript &c, const size_t id)
 {
   try{
     std::stringstream ss;
@@ -67,23 +67,23 @@ int main()
   std::vector<std::shared_ptr<std::thread> > threads;
 
   // Ensure at least two, but say only 7 on an 8 core processor
-  int num_threads = std::max(static_cast<int>(std::thread::hardware_concurrency()) - 1, 2);
+  size_t num_threads = static_cast<size_t>(std::max(static_cast<int>(std::thread::hardware_concurrency()) - 1, 2));
 
   std::cout << "Num threads: " << num_threads << '\n';
 
-  for (int i = 0; i < num_threads; ++i)
+  for (size_t i = 0; i < num_threads; ++i)
   {
     threads.push_back(std::make_shared<std::thread>(do_work, std::ref(chai), i));
   }
 
-  for (int i = 0; i < num_threads; ++i)
+  for (size_t i = 0; i < num_threads; ++i)
   {
     threads[i]->join();
   }
 
 
 
-  for (int i = 0; i < num_threads; ++i)
+  for (size_t i = 0; i < num_threads; ++i)
   {
     std::stringstream ss;
     ss << i;
@@ -92,7 +92,7 @@ int main()
       return EXIT_FAILURE;
     }
 
-    if (chai.eval<int>("getid(" + ss.str() + ")") != i)
+    if (chai.eval<size_t>("getid(" + ss.str() + ")") != i)
     {
       return EXIT_FAILURE;
     }

Original file line number	Diff line number	Diff line change
`@@ -113,6 +113,12 @@ namespace chaiscript {`
`113`	`113`	`#endif`
`114`	`114`	`}`
`115`	`115`
	`116`	`+ template<typename Iter, typename Distance>`
	`117`	`+ Iter advance_copy(Iter iter, Distance distance) {`
	`118`	`+ std::advance(iter, static_cast<typename std::iterator_traits<Iter>::difference_type>(distance));`
	`119`	`+ return iter;`
	`120`	`+ }`
	`121`	`+`
`116`	`122`	`}`
`117`	`123`	`#endif`
`118`	`124`
Original file line number	Diff line number	Diff line change
`@@ -1445,7 +1445,7 @@ namespace chaiscript`
`1445`	`1445`	`static typename Container::const_iterator find_keyed_value(const Container &t_c, const Key &t_key, const size_t t_hint)`
`1446`	`1446`	`{`
`1447`	`1447`	`if (t_c.size() > t_hint && t_c[t_hint].first == t_key) {`
`1448`		`- return t_c.begin() + t_hint;`
	`1448`	`+ return advance_copy(t_c.begin(), t_hint);`
`1449`	`1449`	`} else {`
`1450`	`1450`	`return find_keyed_value(t_c, t_key);`
`1451`	`1451`	`}`
Original file line number	Diff line number	Diff line change
`@@ -505,10 +505,10 @@ namespace chaiscript`
`505`	`505`
`506`	`506`	`// Let's see if this is a link that we should expand`
`507`	`507`	`std::vector<char> buf(2048);`
`508`		`- const size_t pathlen = readlink(dllpath.c_str(), &buf.front(), buf.size());`
`509`		`- if (pathlen > 0 && pathlen < buf.size())`
	`508`	`+ const auto pathlen = readlink(dllpath.c_str(), &buf.front(), buf.size());`
	`509`	`+ if (pathlen > 0 && static_cast<size_t>(pathlen) < buf.size())`
`510`	`510`	`{`
`511`		`- dllpath = std::string(&buf.front(), pathlen);`
	`511`	`+ dllpath = std::string(&buf.front(), static_cast<size_t>(pathlen));`
`512`	`512`	`}`
`513`	`513`
`514`	`514`	`m_module_paths.insert(m_module_paths.begin(), dllpath+"/");`
Original file line number	Diff line number	Diff line change
`@@ -88,24 +88,24 @@ std::vector<std::string> default_search_paths()`
`88`	`88`	`std::vector<char> buf(2048);`
`89`	`89`	`ssize_t size = -1;`
`90`	`90`
`91`		`- if ((size = readlink("/proc/self/exe", &buf.front(), buf.size())) != -1)`
	`91`	`+ if ((size = readlink("/proc/self/exe", &buf.front(), buf.size())) >= 0)`
`92`	`92`	`{`
`93`		`- exepath = std::string(&buf.front(), size);`
	`93`	`+ exepath = std::string(&buf.front(), static_cast<size_t>(size));`
`94`	`94`	`}`
`95`	`95`
`96`	`96`	`if (exepath.empty())`
`97`	`97`	`{`
`98`		`- if ((size = readlink("/proc/curproc/file", &buf.front(), buf.size())) != -1)`
	`98`	`+ if ((size = readlink("/proc/curproc/file", &buf.front(), buf.size())) >= 0)`
`99`	`99`	`{`
`100`		`- exepath = std::string(&buf.front(), size);`
	`100`	`+ exepath = std::string(&buf.front(), static_cast<size_t>(size));`
`101`	`101`	`}`
`102`	`102`	`}`
`103`	`103`
`104`	`104`	`if (exepath.empty())`
`105`	`105`	`{`
`106`		`- if ((size = readlink("/proc/self/path/a.out", &buf.front(), buf.size())) != -1)`
	`106`	`+ if ((size = readlink("/proc/self/path/a.out", &buf.front(), buf.size())) >= 0)`
`107`	`107`	`{`
`108`		`- exepath = std::string(&buf.front(), size);`
	`108`	`+ exepath = std::string(&buf.front(), static_cast<size_t>(size));`
`109`	`109`	`}`
`110`	`110`	`}`
`111`	`111`