Merge pull request #2071 from thk123/refactor/split-string-unit-tests

Enhance split_string to support splitting on white space [TG-2922]

Author: Thomas Kiley

CMakeLists.txt

@@ -75,7 +75,6 @@ set_target_properties(
     mmcc
     pointer-analysis
     solvers
-    string_utils
     test-bigint
     testing-utils
     unit

src/util/string_utils.cpp

@@ -7,11 +7,16 @@ Author: Daniel Poetzl
 \*******************************************************************/
 
 #include "string_utils.h"
+#include "invariant.h"
 
 #include <cassert>
 #include <cctype>
 #include <algorithm>
 
+/// Remove all whitespace characters from either end of a string. Whitespace
+/// in the middle of the string is left unchanged
+/// \param s: the string to strip
+/// \return The stripped string
 std::string strip_string(const std::string &s)
 {
   auto pred=[](char c){ return std::isspace(c); };
@@ -30,15 +35,26 @@ std::string strip_string(const std::string &s)
   return s.substr(i, (j-i+1));
 }
 
+/// Given a string s, split into a sequence of substrings when separated by
+/// specified delimiter.
+/// \param s: The string to split up
+/// \param delim: The character to use as the delimiter
+/// \param [out] result: The sub strings. Must be empty.
+/// \param strip: If true, strip_string will be used on each element, removing
+/// whitespace from the beginning and end of each element
+/// \param remove_empty: If true, all empty-string elements will be removed.
+/// This is applied after strip so whitespace only elements will be removed if
+/// both are set to true
 void split_string(
   const std::string &s,
   char delim,
   std::vector<std::string> &result,
   bool strip,
   bool remove_empty)
 {
-  assert(result.empty());
-  assert(!std::isspace(delim));
+  PRECONDITION(result.empty());
+  // delim can't be a space character if using strip
+  PRECONDITION(!std::isspace(delim) || !strip);
 
   if(s.empty())
   {
@@ -47,7 +63,7 @@ void split_string(
   }
 
   std::string::size_type n=s.length();
-  assert(n>0);
+  INVARIANT(n > 0, "Empty string case should already be handled");
 
   std::string::size_type start=0;
   std::string::size_type i;
@@ -87,7 +103,8 @@ void split_string(
   std::string &right,
   bool strip)
 {
-  assert(!std::isspace(delim));
+  // delim can't be a space character if using strip
+  PRECONDITION(!std::isspace(delim) || !strip);
 
   std::vector<std::string> result;
 

src/util/string_utils.h

@@ -18,10 +18,10 @@ std::string strip_string(const std::string &s);
 
 void split_string(
   const std::string &s,
-  char delim, // must not be a whitespace character
+  char delim,
   std::vector<std::string> &result,
-  bool strip=false, // strip whitespace from elements
-  bool remove_empty=false); // remove empty elements
+  bool strip = false,
+  bool remove_empty = false);
 
 void split_string(
   const std::string &s,

unit/.gitignore

@@ -1,5 +1,4 @@
 # Unit test binaries
 miniBDD
 sharing_node
-string_utils
 unit_tests

unit/CMakeLists.txt

@@ -5,7 +5,6 @@ file(GLOB_RECURSE testing_utils "testing-utils/*.cpp" "testing-utils/*.h")
 list(REMOVE_ITEM sources
     # Used in executables
     ${CMAKE_CURRENT_SOURCE_DIR}/miniBDD.cpp
-    ${CMAKE_CURRENT_SOURCE_DIR}/string_utils.cpp
 
     # Don't build
     ${CMAKE_CURRENT_SOURCE_DIR}/sharing_map.cpp
@@ -63,14 +62,3 @@ target_include_directories(miniBDD
 target_link_libraries(miniBDD solvers ansi-c)
 add_test(NAME miniBDD COMMAND $<TARGET_FILE:miniBDD>)
 set_tests_properties(miniBDD PROPERTIES LABELS "CORE;CBMC")
-
-add_executable(string_utils string_utils.cpp)
-target_include_directories(string_utils
-    PUBLIC
-    ${CBMC_BINARY_DIR}
-    ${CBMC_SOURCE_DIR}
-    ${CMAKE_CURRENT_SOURCE_DIR}
-)
-target_link_libraries(string_utils solvers ansi-c)
-add_test(NAME string_utils COMMAND $<TARGET_FILE:string_utils>)
-set_tests_properties(string_utils PROPERTIES LABELS "CORE;CBMC")

unit/Makefile

@@ -3,10 +3,6 @@
 # Source files for test utilities
 SRC = unit_tests.cpp \
       catch_example.cpp \
-      util/expr_iterator.cpp \
-      util/optional.cpp \
-      analyses/call_graph.cpp \
-      java_bytecode/java_bytecode_convert_class/convert_abstract_class.cpp \
       # Empty last line
 
 # Test source files
@@ -48,8 +44,11 @@ SRC += unit_tests.cpp \
        util/irep.cpp \
        util/irep_sharing.cpp \
        util/message.cpp \
+       util/optional.cpp \
        util/parameter_indices.cpp \
        util/simplify_expr.cpp \
+       util/string_utils/split_string.cpp \
+       util/string_utils/strip_string.cpp \
        util/symbol_table.cpp \
        catch_example.cpp \
        java_bytecode/java_virtual_functions/virtual_functions.cpp \
@@ -88,7 +87,6 @@ OBJ += $(CPROVER_LIBS) testing-utils/testing-utils$(LIBEXT)
 
 TESTS = unit_tests$(EXEEXT) \
         miniBDD$(EXEEXT) \
-        string_utils$(EXEEXT) \
         # Empty last line
 
 CLEANFILES = $(TESTS)
@@ -107,6 +105,3 @@ unit_tests$(EXEEXT): $(OBJ)
 
 miniBDD$(EXEEXT): miniBDD$(OBJEXT) $(CPROVER_LIBS)
 	$(LINKBIN)
-
-string_utils$(EXEEXT): string_utils$(OBJEXT) $(CPROVER_LIBS)
-	$(LINKBIN)