From a6719f47e595bd410168de2d76c46dc36e0432d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Klinkovsk=C3=BD?= <klinkjak@fjfi.cvut.cz>
Date: Wed, 28 Dec 2016 14:44:35 +0100
Subject: [PATCH] Fixed bug in parseObjectType function and added tests

The original algorithm used space as one of the separators between types
in template parameter lists, so as a result "A< short int >" was parsed
into a list of ("A", "short", "int").
---
 src/TNL/Containers/List.h      |  4 ++
 src/TNL/Containers/List_impl.h | 17 ++++++++
 src/TNL/Object.cpp             |  7 ++--
 src/TNL/String.cpp             | 17 ++++++++
 src/TNL/String.h               |  2 +
 src/UnitTests/ObjectTest.cpp   | 72 ++++++++++++++++++++++++++++++++++
 src/UnitTests/StringTest.cpp   | 11 ++++++
 7 files changed, 126 insertions(+), 4 deletions(-)

diff --git a/src/TNL/Containers/List.h b/src/TNL/Containers/List.h
index 3692a9a2ca..adb163374d 100644
--- a/src/TNL/Containers/List.h
+++ b/src/TNL/Containers/List.h
@@ -71,6 +71,10 @@ template< class T > class List
 
       const List& operator = ( const List& lst );
 
+      bool operator == ( const List& lst ) const;
+
+      bool operator != ( const List& lst ) const;
+
       //! Append new data element
       bool Append( const T& data );
 
diff --git a/src/TNL/Containers/List_impl.h b/src/TNL/Containers/List_impl.h
index 63f5f195dd..f91f30ca2b 100644
--- a/src/TNL/Containers/List_impl.h
+++ b/src/TNL/Containers/List_impl.h
@@ -107,6 +107,23 @@ const List< T >& List< T >::operator = ( const List& lst )
    return( *this );
 }
 
+template< typename T >
+bool List< T >::operator == ( const List& lst ) const
+{
+   if( this->getSize() != lst.getSize() )
+      return false;
+   for( int i = 0; i < this->getSize(); i++ )
+      if( (*this)[ i ] != lst[ i ] )
+         return false;
+   return true;
+}
+
+template< typename T >
+bool List< T >::operator != ( const List& lst ) const
+{
+   return ! operator==( lst );
+}
+
 template< typename T >
 bool List< T >::Append( const T& data )
 {
diff --git a/src/TNL/Object.cpp b/src/TNL/Object.cpp
index c5350b287a..14418d26da 100644
--- a/src/TNL/Object.cpp
+++ b/src/TNL/Object.cpp
@@ -189,7 +189,7 @@ bool parseObjectType( const String& objectType,
 
    /****
     * Now, we will extract the parameters.
-    * Each parameter can be template, so we must compute and pair
+    * Each parameter can be template, so we must count and pair
     * '<' with '>'.
     */
    int templateBrackets( 0 );
@@ -201,13 +201,12 @@ bool parseObjectType( const String& objectType,
          templateBrackets ++;
       if( ! templateBrackets )
       {
-         if( objectType[ i ] == ' ' ||
-             objectType[ i ] == ',' ||
+         if( objectType[ i ] == ',' ||
              objectType[ i ] == '>' )
          {
             if( buffer != "" )
             {
-               if( ! parsedObjectType. Append( buffer ) )
+               if( ! parsedObjectType. Append( buffer.strip( ' ' ) ) )
                   return false;
                buffer. setString( "" );
             }
diff --git a/src/TNL/String.cpp b/src/TNL/String.cpp
index 4607d742a9..980e0a1f60 100644
--- a/src/TNL/String.cpp
+++ b/src/TNL/String.cpp
@@ -269,6 +269,23 @@ replace( const String& pattern,
    this->string = newString;
 }
 
+String
+String::strip( char strip ) const
+{
+   int prefix_cut_off = 0;
+   int sufix_cut_off = 0;
+
+   while( prefix_cut_off < getLength() && (*this)[ prefix_cut_off ] == strip )
+      prefix_cut_off++;
+
+   while( sufix_cut_off < getLength() && (*this)[ getLength() - 1 - sufix_cut_off ] == strip )
+      sufix_cut_off++;
+
+   if( prefix_cut_off + sufix_cut_off < getLength() )
+      return String( getString(), prefix_cut_off, sufix_cut_off );
+   return "";
+}
+
 
 const char* String :: getString() const
 {
diff --git a/src/TNL/String.h b/src/TNL/String.h
index 254c283e2c..65daaf4c8f 100644
--- a/src/TNL/String.h
+++ b/src/TNL/String.h
@@ -128,6 +128,8 @@ class String
    void replace( const String& pattern,
                  const String& replaceWith );
 
+   String strip( char strip = ' ' ) const;
+
    // TODO: remove
    //! Write to a binary file
    bool save( std::ostream& file ) const;
diff --git a/src/UnitTests/ObjectTest.cpp b/src/UnitTests/ObjectTest.cpp
index 557d1239ba..3c495846d3 100644
--- a/src/UnitTests/ObjectTest.cpp
+++ b/src/UnitTests/ObjectTest.cpp
@@ -11,6 +11,7 @@
 #include <TNL/Devices/Host.h>
 #include <TNL/Object.h>
 #include <TNL/File.h>
+#include <TNL/Containers/Array.h>
 
 #ifdef HAVE_GTEST 
 #include "gtest/gtest.h"
@@ -29,6 +30,77 @@ TEST( ObjectTest, SaveAndLoadTest )
    file.open( "test-file.tnl", tnlReadMode );
    ASSERT_TRUE( testObject.load( file ) );
 }
+
+TEST( ObjectTest, parseObjectTypeTest )
+{
+   Containers::List< String > parsed;
+   Containers::List< String > expected;
+
+   // plain type
+   parsed.reset();
+   expected.reset();
+   ASSERT_TRUE( parseObjectType( "int", parsed ) );
+   expected.Append( "int" );
+   EXPECT_EQ( parsed, expected );
+
+   // type with space
+   parsed.reset();
+   expected.reset();
+   ASSERT_TRUE( parseObjectType( "short int", parsed ) );
+   expected.Append( "short int" );
+   EXPECT_EQ( parsed, expected );
+
+   parsed.reset();
+   expected.reset();
+   ASSERT_TRUE( parseObjectType( "unsigned short int", parsed ) );
+   expected.Append( "unsigned short int" );
+   EXPECT_EQ( parsed, expected );
+
+   // composed type
+   parsed.reset();
+   expected.reset();
+   ASSERT_TRUE( parseObjectType( "Containers::Vector< double, Devices::Host, int >", parsed ) );
+   expected.Append( "Containers::Vector" );
+   expected.Append( "double" );
+   expected.Append( "Devices::Host" );
+   expected.Append( "int" );
+   EXPECT_EQ( parsed, expected );
+
+   parsed.reset();
+   expected.reset();
+   ASSERT_TRUE( parseObjectType( "Containers::Vector< Containers::List< String >, Devices::Host, int >", parsed ) );
+   expected.Append( "Containers::Vector" );
+   expected.Append( "Containers::List< String >" );
+   expected.Append( "Devices::Host" );
+   expected.Append( "int" );
+   EXPECT_EQ( parsed, expected );
+
+   // spaces in the template parameter
+   parsed.reset();
+   expected.reset();
+   ASSERT_TRUE( parseObjectType( "A< short int >", parsed ) );
+   expected.Append( "A" );
+   expected.Append( "short int" );
+   EXPECT_EQ( parsed, expected );
+
+   parsed.reset();
+   expected.reset();
+   ASSERT_TRUE( parseObjectType( "A< B< short int >, C >", parsed ) );
+   expected.Append( "A" );
+   expected.Append( "B< short int >" );
+   expected.Append( "C" );
+   EXPECT_EQ( parsed, expected );
+
+   // spaces at different places in the template parameter
+   parsed.reset();
+   expected.reset();
+   ASSERT_TRUE( parseObjectType( "A< b , c <E>  ,d>", parsed ) );
+   expected.Append( "A" );
+   expected.Append( "b" );
+   expected.Append( "c <E>" );
+   expected.Append( "d" );
+   EXPECT_EQ( parsed, expected );
+}
 #endif
 
 
diff --git a/src/UnitTests/StringTest.cpp b/src/UnitTests/StringTest.cpp
index 9b76b0f6d4..1063d139dc 100644
--- a/src/UnitTests/StringTest.cpp
+++ b/src/UnitTests/StringTest.cpp
@@ -106,6 +106,17 @@ TEST( StringTest, AdditionAssignmentOperator )
    ASSERT_EQ( strcmp( string2. getString(), "stringstring2" ), 0 );
 }
 
+TEST( StringTest, strip )
+{
+   EXPECT_EQ( String( "string" ).strip(), String( "string" ) );
+   EXPECT_EQ( String( "  string" ).strip(), String( "string" ) );
+   EXPECT_EQ( String( "string  " ).strip(), String( "string" ) );
+   EXPECT_EQ( String( "  string  " ).strip(), String( "string" ) );
+   EXPECT_EQ( String( " string1  string2  " ).strip(), String( "string1  string2" ) );
+   EXPECT_EQ( String( "" ).strip(), String( "" ) );
+   EXPECT_EQ( String( "  " ).strip(), String( "" ) );
+}
+
 
 TEST( StringTest, SaveLoad )
 {
-- 
GitLab