Fixed bug 4976: optional and string literals

2 files changed, 83 insertions, 41 deletions

diff --git a/cpp/include/IceUtil/Optional.h b/cpp/include/IceUtil/Optional.h
index f6f2613fd08..fd9fa3b4de4 100644
--- a/cpp/include/IceUtil/Optional.h
+++ b/cpp/include/IceUtil/Optional.h
@@ -41,16 +41,13 @@ public:
     {
     }
 
-    Optional(const T& p) : _value(p), _isSet(true)
-    {
-    }
-    
     template<typename Y>
-    Optional(const Optional<Y>& r) : _value(r._value), _isSet(r._isSet)
+    Optional(Y p) : _value(p), _isSet(true)
     {
     }
 
-    Optional(const Optional& r) : _value(r._value), _isSet(r._isSet)
+    template<typename Y>
+    Optional(const Optional<Y>& r) : _value(r._value), _isSet(r._isSet)
     {
     }
     
@@ -65,7 +62,8 @@ public:
         return *this;
     }
 
-    Optional& operator=(const T& p)
+    template<typename Y>
+    Optional& operator=(Y p)
     {
         _value = p;
         _isSet = true;
@@ -172,10 +170,10 @@ inline bool operator==(const Optional<T>& lhs, const Optional<U>& rhs)
     {
         return *lhs == *rhs;
     }
-
-    // Note: don't use if { } else { }. This causes lots warnings when
-    // compiling with GCC and optimization enabled. See bug 2330.
-    return !lhs && !rhs;
+    else
+    {
+        return !lhs && !rhs;
+    }
 }
 
 template<typename T, typename U>
@@ -191,10 +189,10 @@ inline bool operator<(const Optional<T>& lhs, const Optional<U>& rhs)
     {
         return *lhs < *rhs;
     }
-
-    // Note: don't use if { } else { }. This causes lots warnings when
-    // compiling with GCC and optimization enabled. See bug 2330.
-    return !lhs && rhs;
+    else
+    {
+        return !lhs && rhs;
+    }
 }
 
 template<typename T, typename U>
@@ -220,75 +218,103 @@ inline bool operator>=(const Optional<T>& lhs, const Optional<U>& rhs)
 template<typename T, typename Y>
 inline bool operator==(const Optional<T>& lhs, const Y& rhs)
 {
-    return operator==(lhs, Optional<Y>(rhs));
+    if(!lhs)
+    {
+        return false;
+    }
+    else 
+    {
+        return *lhs == rhs;
+    }
 }
 
 template<typename T, typename Y>
 inline bool operator!=(const Optional<T>& lhs, const Y& rhs)
 {
-    return !operator==(lhs, Optional<Y>(rhs));
+    return !operator==(lhs, rhs);
 }
 
 template<typename T, typename Y>
 inline bool operator<(const Optional<T>& lhs, const Y& rhs)
 {
-    return operator<(lhs, Optional<Y>(rhs));
+    if(lhs)
+    {
+        return *lhs < rhs;
+    }
+    else
+    {
+        return true;
+    }
 }
 
 template<typename T, typename Y>
 inline bool operator<=(const Optional<T>& lhs, const Y& rhs)
 {
-    return operator<=(lhs, Optional<Y>(rhs));
+    return lhs < rhs || lhs == rhs;
 }
 
 template<typename T, typename Y>
 inline bool operator>(const Optional<T>& lhs, const Y& rhs)
 {
-    return operator>(lhs, Optional<Y>(rhs));
+    return !(lhs < rhs || lhs == rhs);
 }
 
 template<typename T, typename Y>
 inline bool operator>=(const Optional<T>& lhs, const Y& rhs)
 {
-    return operator>=(lhs, Optional<Y>(rhs));
+    return !(lhs < rhs);
 }
 
 // Y vs Optional<T>
 
 template<typename T, typename Y>
-inline bool operator==(const Y& lhs, const Optional<Y>& rhs)
+inline bool operator==(const Y& lhs, const Optional<T>& rhs)
 {
-    return operator==(Optional<Y>(lhs), rhs);
+    if(!rhs)
+    {
+        return false;
+    }
+    else 
+    {
+        return lhs == *rhs;
+    }
 }
 
 template<typename T, typename Y>
-inline bool operator!=(const Y& lhs, const Optional<Y>& rhs)
+inline bool operator!=(const Y& lhs, const Optional<T>& rhs)
 {
-    return !operator==(Optional<Y>(lhs), rhs);
+    return !operator==(lhs, rhs);
 }
 
 template<typename T, typename Y>
-inline bool operator<(const Y& lhs, const Optional<Y>& rhs)
+inline bool operator<(const Y& lhs, const Optional<T>& rhs)
 {
-    return operator<(Optional<Y>(lhs), rhs);
+    if(rhs)
+    {
+        return lhs < *rhs;
+    }
+    else
+    {
+        return false;
+    }
 }
 
 template<typename T, typename Y>
-inline bool operator<=(const Y& lhs, const Optional<Y>& rhs)
+inline bool operator<=(const Y& lhs, const Optional<T>& rhs)
 {
-    return operator<=(Optional<Y>(lhs), rhs);
+    return lhs < rhs || lhs == rhs;
 }
 
 template<typename T, typename Y>
-inline bool operator>(const Y& lhs, const Optional<Y>& rhs)
+inline bool operator>(const Y& lhs, const Optional<T>& rhs)
 {
-    return operator>(Optional<Y>(lhs), rhs);
+    return !(lhs < rhs || lhs == rhs);
 }
 
 template<typename T, typename Y>
-inline bool operator>=(const Y& lhs, const Optional<Y>& rhs)
+inline bool operator>=(const Y& lhs, const Optional<T>& rhs)
 {
-    return operator>=(Optional<Y>(lhs), rhs);
+    return !(lhs < rhs);
 }
 
 }
diff --git a/cpp/test/Ice/optional/AllTests.cpp b/cpp/test/Ice/optional/AllTests.cpp
index 6719dca601e..f00294d28e5 100644
--- a/cpp/test/Ice/optional/AllTests.cpp
+++ b/cpp/test/Ice/optional/AllTests.cpp
@@ -310,7 +310,7 @@ allTests(const Ice::CommunicatorPtr& communicator, bool collocated)
     test(mo3->e == 99);
     test(mo3->f == 5.5f);
     test(mo3->g == 1.0);
-    test(mo3->h == string("test"));
+    test(mo3->h == "test");
     test(mo3->i == Test::MyEnumMember);
     test(mo3->j == MultiOptionalPrx::uncheckedCast(communicator->stringToProxy("test")));
     test(mo3->k == mo1);
@@ -338,6 +338,22 @@ allTests(const Ice::CommunicatorPtr& communicator, bool collocated)
 
     cout << "ok" << endl;
 
+    cout << "testing comparison operators... " << flush;
+
+    test(mo1->a == 15 && 15 == mo1->a && mo1->a != 16 && 16 != mo1->a);
+    test(mo1->a < 16 && mo1->a > 14 && mo1->a <= 15 && mo1->a >= 15 && mo1->a <= 16 && mo1->a >= 14);
+    test(mo1->a > IceUtil::Optional<int>() && IceUtil::Optional<int>() < mo1->a);
+    test(14 > IceUtil::Optional<int>() && IceUtil::Optional<int>() < 14);
+
+    test(mo1->h == "test" && "test" == mo1->h && mo1->h != "testa" && "testa" != mo1->h);
+    test(mo1->h < "test1" && mo1->h > "tesa" && mo1->h <= "test");
+    test(mo1->h >= "test" && mo1->h <= "test1" && mo1->h >= "tesa");
+    test(mo1->h > IceUtil::Optional<string>() && IceUtil::Optional<string>() < mo1->h);
+    test("test1" > IceUtil::Optional<string>() && IceUtil::Optional<string>() < "test1");
+
+    cout << "ok" << endl;
+
+
     cout << "testing marshalling... " << flush;
     OneOptionalPtr oo4 = OneOptionalPtr::dynamicCast(initial->pingPong(new OneOptional()));
     test(!oo4->a);
@@ -773,9 +789,9 @@ allTests(const Ice::CommunicatorPtr& communicator, bool collocated)
         IceUtil::Optional<string> p2 = initial->opString(p1, p3);
         test(!p2 && !p3);
 
-        p1 = string("test");
-        p2 = initial->opString(p1, p3);
-        test(p2 == string("test") && p3 == string("test"));
+        p1 = "test";
+        p2 = initial->opString("test", p3);
+        test(p2 == "test" && p3 == "test");
 
         out = Ice::createOutputStream(communicator);
         out->startEncapsulation();
@@ -788,7 +804,7 @@ allTests(const Ice::CommunicatorPtr& communicator, bool collocated)
         in->read(1, p2);
         in->read(3, p3);
         in->endEncapsulation();
-        test(p2 == string("test") && p3 == string("test"));
+        test(p2 == "test" && p3 == "test");
 
         in = Ice::createInputStream(communicator, outEncaps);
         in->startEncapsulation();
@@ -1097,12 +1113,12 @@ allTests(const Ice::CommunicatorPtr& communicator, bool collocated)
 
         try
         {
-            initial->opOptionalException(30, string("test"), OneOptionalPtr(new OneOptional(53)));
+            initial->opOptionalException(30, "test", new OneOptional(53));
         }
         catch(const OptionalException& ex)
         {
             test(ex.a == 30);
-            test(ex.b == string("test"));
+            test(ex.b == "test");
             test((*ex.o)->a = 53);
         }
 
@@ -1112,7 +1128,7 @@ allTests(const Ice::CommunicatorPtr& communicator, bool collocated)
             // Use the 1.0 encoding with an exception whose only class members are optional.
             //
             initial->ice_encodingVersion(Ice::Encoding_1_0)->
-                opOptionalException(30, string("test"), OneOptionalPtr(new OneOptional(53)));
+                opOptionalException(30, "test", new OneOptional(53));
         }
         catch(const OptionalException& ex)
         {