Fixed bug #ICE-4845 (ice_clone for IceUtil::Exception is now covariant)

Small build fixes (VS warning, Python 3.x run.py)
slice2cpp no longer generates ice_clone for interfaces

20 files changed, 71 insertions, 61 deletions

diff --git a/cpp/src/Freeze/TransactionalEvictorContext.cpp b/cpp/src/Freeze/TransactionalEvictorContext.cpp
index c65523fa836..b0dacafe261 100644
--- a/cpp/src/Freeze/TransactionalEvictorContext.cpp
+++ b/cpp/src/Freeze/TransactionalEvictorContext.cpp
@@ -39,7 +39,7 @@ Freeze::TransactionalEvictorDeadlockException::ice_name() const
     return "Freeze::TransactionalEvictorDeadlockException";
 }
 
-Ice::Exception*
+Freeze::TransactionalEvictorDeadlockException*
 Freeze::TransactionalEvictorDeadlockException::ice_clone() const
 {
     return new TransactionalEvictorDeadlockException(*this);
@@ -197,7 +197,7 @@ Freeze::TransactionalEvictorContext::exception(const std::exception& ex)
 
     if(dx != 0 && _owner == IceUtil::ThreadControl())
     {
-        _deadlockException.reset(dynamic_cast<DeadlockException*>(dx->ice_clone()));
+        _deadlockException.reset(dx->ice_clone());
         return false;
     }
 
@@ -205,7 +205,7 @@ Freeze::TransactionalEvictorContext::exception(const std::exception& ex)
         dynamic_cast<const TransactionalEvictorDeadlockException*>(&ex);
     if(edx != 0 && _owner == IceUtil::ThreadControl())
     {
-        _nestedCallDeadlockException.reset(dynamic_cast<TransactionalEvictorDeadlockException*>(edx->ice_clone()));
+        _nestedCallDeadlockException.reset(edx->ice_clone());
         return false;
     }
 
diff --git a/cpp/src/Glacier2Lib/Application.cpp b/cpp/src/Glacier2Lib/Application.cpp
index 8b62671a3d9..ee748dcae32 100644
--- a/cpp/src/Glacier2Lib/Application.cpp
+++ b/cpp/src/Glacier2Lib/Application.cpp
@@ -140,7 +140,7 @@ Glacier2::RestartSessionException::ice_name() const
     return "RestartSessionException";
 }
 
-Exception*
+Glacier2::RestartSessionException*
 Glacier2::RestartSessionException::ice_clone() const
 {
     return new RestartSessionException(*this);
diff --git a/cpp/src/Glacier2Lib/SessionHelper.cpp b/cpp/src/Glacier2Lib/SessionHelper.cpp
index 046cc984d11..e5a25ad4095 100644
--- a/cpp/src/Glacier2Lib/SessionHelper.cpp
+++ b/cpp/src/Glacier2Lib/SessionHelper.cpp
@@ -517,7 +517,7 @@ public:
         _callback(callback),
         _session(session)
     {
-        _ex.reset(dynamic_cast<Ice::Exception*>(ex.ice_clone()));
+        _ex.reset(ex.ice_clone());
     }
     
     virtual void
diff --git a/cpp/src/Ice/ConnectRequestHandler.cpp b/cpp/src/Ice/ConnectRequestHandler.cpp
index a0a900e4b52..de6d765137d 100644
--- a/cpp/src/Ice/ConnectRequestHandler.cpp
+++ b/cpp/src/Ice/ConnectRequestHandler.cpp
@@ -34,7 +34,7 @@ public:
                                const Ice::LocalException& ex) :
         DispatchWorkItem(instance),
         _handler(handler),
-        _exception(dynamic_cast<Ice::LocalException*>(ex.ice_clone()))
+        _exception(ex.ice_clone())
     {
     }
     
@@ -328,7 +328,7 @@ ConnectRequestHandler::setException(const Ice::LocalException& ex)
     assert(!_initialized && !_exception.get());
     assert(_updateRequestHandler || _requests.empty());
 
-    _exception.reset(dynamic_cast<Ice::LocalException*>(ex.ice_clone()));
+    _exception.reset(ex.ice_clone());
     _proxy = 0; // Break cyclic reference count.
     _delegate = 0; // Break cyclic reference count.
 
@@ -451,7 +451,7 @@ ConnectRequestHandler::flushRequests()
     {
         Lock sync(*this);
         assert(!_exception.get() && !_requests.empty());
-        _exception.reset(dynamic_cast<Ice::LocalException*>(ex.get()->ice_clone()));
+        _exception.reset(ex.get()->ice_clone());
         const InstancePtr instance = _reference->getInstance();
         instance->clientThreadPool()->execute(new FlushRequestsWithExceptionWrapper(instance, this, ex));
     }
@@ -459,7 +459,7 @@ ConnectRequestHandler::flushRequests()
     {
         Lock sync(*this);
         assert(!_exception.get() && !_requests.empty());
-        _exception.reset(dynamic_cast<Ice::LocalException*>(ex.ice_clone()));
+        _exception.reset(ex.ice_clone());
         const InstancePtr instance = _reference->getInstance();
         instance->clientThreadPool()->execute(new FlushRequestsWithException(instance, this, ex));
     }
diff --git a/cpp/src/Ice/ConnectionFactory.cpp b/cpp/src/Ice/ConnectionFactory.cpp
index 035e8ad04c7..a607fa928e2 100755
--- a/cpp/src/Ice/ConnectionFactory.cpp
+++ b/cpp/src/Ice/ConnectionFactory.cpp
@@ -196,7 +196,7 @@ IceInternal::OutgoingConnectionFactory::create(const vector<EndpointIPtr>& endpt
         }
         catch(const Ice::LocalException& ex)
         {
-            exception.reset(dynamic_cast<Ice::LocalException*>(ex.ice_clone()));
+            exception.reset(ex.ice_clone());
             handleException(ex, hasMore || p != endpoints.end() - 1);
         }
     }
@@ -245,14 +245,14 @@ IceInternal::OutgoingConnectionFactory::create(const vector<EndpointIPtr>& endpt
         }
         catch(const Ice::CommunicatorDestroyedException& ex)
         {
-            exception.reset(dynamic_cast<Ice::LocalException*>(ex.ice_clone()));
+            exception.reset(ex.ice_clone());
             handleConnectionException(*exception.get(), hasMore || q != connectors.end() - 1);
             connection = 0;
             break; // No need to continue
         }
         catch(const Ice::LocalException& ex)
         {
-            exception.reset(dynamic_cast<Ice::LocalException*>(ex.ice_clone()));
+            exception.reset(ex.ice_clone());
             handleConnectionException(*exception.get(), hasMore || q != connectors.end() - 1);
             connection = 0;
         }
diff --git a/cpp/src/Ice/ConnectionI.cpp b/cpp/src/Ice/ConnectionI.cpp
index e1a8e03dd82..7f04c78019e 100644
--- a/cpp/src/Ice/ConnectionI.cpp
+++ b/cpp/src/Ice/ConnectionI.cpp
@@ -1866,7 +1866,7 @@ Ice::ConnectionI::setState(State state, const LocalException& ex)
         //
         assert(_state != StateClosed);
 
-        _exception.reset(dynamic_cast<LocalException*>(ex.ice_clone()));
+        _exception.reset(ex.ice_clone());
 
         if(_warn)
         {
diff --git a/cpp/src/Ice/Direct.cpp b/cpp/src/Ice/Direct.cpp
index 64f61790abe..9c4d906d9ec 100644
--- a/cpp/src/Ice/Direct.cpp
+++ b/cpp/src/Ice/Direct.cpp
@@ -107,7 +107,7 @@ IceInternal::Direct::throwUserException()
 void 
 IceInternal::Direct::setUserException(const Ice::UserException& ue)
 {
-    _userException.reset(dynamic_cast<Ice::UserException*>(ue.ice_clone()));
+    _userException.reset(ue.ice_clone());
 }
 
 void
diff --git a/cpp/src/Ice/Object.cpp b/cpp/src/Ice/Object.cpp
index a8686c343bf..096bf434b5f 100644
--- a/cpp/src/Ice/Object.cpp
+++ b/cpp/src/Ice/Object.cpp
@@ -86,7 +86,7 @@ Ice::Object::ice_staticId()
     return __Ice__Object_ids[0];
 }
 
-ObjectPtr
+Ice::ObjectPtr
 Ice::Object::ice_clone() const
 {
     throw CloneNotImplementedException(__FILE__, __LINE__);
diff --git a/cpp/src/Ice/Outgoing.cpp b/cpp/src/Ice/Outgoing.cpp
index e922fb30486..a136e2f3b38 100644
--- a/cpp/src/Ice/Outgoing.cpp
+++ b/cpp/src/Ice/Outgoing.cpp
@@ -25,13 +25,13 @@ using namespace IceInternal;
 IceInternal::LocalExceptionWrapper::LocalExceptionWrapper(const LocalException& ex, bool r) :
     _retry(r)
 {
-    _ex.reset(dynamic_cast<LocalException*>(ex.ice_clone()));
+    _ex.reset(ex.ice_clone());
 }
 
 IceInternal::LocalExceptionWrapper::LocalExceptionWrapper(const LocalExceptionWrapper& ex) :
     _retry(ex._retry)
 {
-    _ex.reset(dynamic_cast<LocalException*>(ex.get()->ice_clone()));
+    _ex.reset(ex.get()->ice_clone());
 }
 
 void
@@ -511,7 +511,7 @@ IceInternal::Outgoing::finished(const LocalException& ex, bool sent)
     IceUtil::Monitor<IceUtil::Mutex>::Lock sync(_monitor);
     assert(_state <= StateInProgress);
     _state = StateFailed;
-    _exception.reset(dynamic_cast<LocalException*>(ex.ice_clone()));
+    _exception.reset(ex.ice_clone());
     _sent = sent;
     _monitor.notify();
 }
@@ -585,6 +585,6 @@ void
 IceInternal::BatchOutgoing::finished(const Ice::LocalException& ex, bool)
 {
     IceUtil::Monitor<IceUtil::Mutex>::Lock sync(_monitor);
-    _exception.reset(dynamic_cast<LocalException*>(ex.ice_clone()));
+    _exception.reset(ex.ice_clone());
     _monitor.notify();
 }
diff --git a/cpp/src/Ice/OutgoingAsync.cpp b/cpp/src/Ice/OutgoingAsync.cpp
index a5bfa8c4af8..e019e9e4949 100644
--- a/cpp/src/Ice/OutgoingAsync.cpp
+++ b/cpp/src/Ice/OutgoingAsync.cpp
@@ -55,7 +55,7 @@ public:
     AsynchronousException(const IceInternal::InstancePtr& instance,
                           const Ice::AsyncResultPtr& result,
                           const Ice::Exception& ex) :
-        DispatchWorkItem(instance), _result(result), _exception(dynamic_cast<Ice::LocalException*>(ex.ice_clone()))
+        DispatchWorkItem(instance), _result(result), _exception(ex.ice_clone())
     {
     }
 
diff --git a/cpp/src/Ice/Reference.cpp b/cpp/src/Ice/Reference.cpp
index 0b8a6eaff7b..1d5f4dd1b91 100644
--- a/cpp/src/Ice/Reference.cpp
+++ b/cpp/src/Ice/Reference.cpp
@@ -1689,7 +1689,7 @@ IceInternal::RoutableReference::createConnection(const vector<EndpointIPtr>& all
             }
             catch(const LocalException& ex)
             {
-                exception.reset(dynamic_cast<LocalException*>(ex.ice_clone()));
+                exception.reset(ex.ice_clone());
             }
         }
 
@@ -1801,7 +1801,7 @@ IceInternal::RoutableReference::createConnection(const vector<EndpointIPtr>& all
             {
                 if(!_exception.get())
                 {
-                    _exception.reset(dynamic_cast<Ice::LocalException*>(ex.ice_clone()));
+                    _exception.reset(ex.ice_clone());
                 }
                 
                 if(++_i == _endpoints.size())
diff --git a/cpp/src/IceSSL/Certificate.cpp b/cpp/src/IceSSL/Certificate.cpp
index 5a33fdda464..da9e7e2c4b9 100644
--- a/cpp/src/IceSSL/Certificate.cpp
+++ b/cpp/src/IceSSL/Certificate.cpp
@@ -40,7 +40,7 @@ CertificateReadException::ice_name() const
     return _name;
 }
 
-Exception* 
+CertificateReadException* 
 CertificateReadException::ice_clone() const
 {
     return new CertificateReadException(*this);
@@ -70,7 +70,7 @@ CertificateEncodingException::ice_name() const
     return _name;
 }
 
-Exception* 
+CertificateEncodingException* 
 CertificateEncodingException::ice_clone() const
 {
     return new CertificateEncodingException(*this);
@@ -269,7 +269,7 @@ ParseException::ice_name() const
     return _name;
 }
 
-IceUtil::Exception* 
+ParseException* 
 ParseException::ice_clone() const
 {
     return new ParseException(*this);
diff --git a/cpp/src/IceUtil/CtrlCHandler.cpp b/cpp/src/IceUtil/CtrlCHandler.cpp
index 26b9fcf711a..bb6bbb9f2a3 100644
--- a/cpp/src/IceUtil/CtrlCHandler.cpp
+++ b/cpp/src/IceUtil/CtrlCHandler.cpp
@@ -68,7 +68,7 @@ CtrlCHandlerException::ice_name() const
     return ctrlCHandlerName;
 }
 
-Exception*
+CtrlCHandlerException*
 CtrlCHandlerException::ice_clone() const
 {
     return new CtrlCHandlerException(*this);
diff --git a/cpp/src/IceUtil/Exception.cpp b/cpp/src/IceUtil/Exception.cpp
index b6c75231f5a..c310b2fcb03 100644
--- a/cpp/src/IceUtil/Exception.cpp
+++ b/cpp/src/IceUtil/Exception.cpp
@@ -300,7 +300,7 @@ IceUtil::NullHandleException::ice_name() const
     return _name;
 }
 
-IceUtil::Exception*
+IceUtil::NullHandleException*
 IceUtil::NullHandleException::ice_clone() const
 {
     return new NullHandleException(*this);
@@ -342,7 +342,7 @@ IceUtil::IllegalArgumentException::ice_print(ostream& out) const
     out << ": " << _reason;
 }
 
-IceUtil::Exception*
+IceUtil::IllegalArgumentException*
 IceUtil::IllegalArgumentException::ice_clone() const
 {
     return new IllegalArgumentException(*this);
@@ -384,7 +384,7 @@ IceUtil::SyscallException::ice_print(ostream& os) const
     }
 }
 
-IceUtil::Exception*
+IceUtil::SyscallException*
 IceUtil::SyscallException::ice_clone() const
 {
     return new SyscallException(*this);
@@ -433,7 +433,7 @@ IceUtil::FileLockException::ice_print(ostream& os) const
     }
 }
 
-IceUtil::Exception*
+IceUtil::FileLockException*
 IceUtil::FileLockException::ice_clone() const
 {
     return new FileLockException(*this);
@@ -472,7 +472,7 @@ IceUtil::OptionalNotSetException::ice_name() const
     return _name;
 }
 
-IceUtil::Exception*
+IceUtil::OptionalNotSetException*
 IceUtil::OptionalNotSetException::ice_clone() const
 {
     return new OptionalNotSetException(*this);
diff --git a/cpp/src/IceUtil/Options.cpp b/cpp/src/IceUtil/Options.cpp
index b632d0e90fa..32840de107c 100644
--- a/cpp/src/IceUtil/Options.cpp
+++ b/cpp/src/IceUtil/Options.cpp
@@ -42,7 +42,7 @@ IceUtilInternal::APIException::ice_print(ostream& out) const
     }
 }
 
-IceUtil::Exception*
+IceUtilInternal::APIException*
 IceUtilInternal::APIException::ice_clone() const
 {
     return new APIException(*this);
@@ -88,7 +88,7 @@ IceUtilInternal::BadOptException::ice_print(ostream& out) const
     }
 }
 
-IceUtil::Exception*
+IceUtilInternal::BadOptException*
 IceUtilInternal::BadOptException::ice_clone() const
 {
     return new BadOptException(*this);
diff --git a/cpp/src/IceUtil/ThreadException.cpp b/cpp/src/IceUtil/ThreadException.cpp
index dcfcfad386e..f034da27408 100644
--- a/cpp/src/IceUtil/ThreadException.cpp
+++ b/cpp/src/IceUtil/ThreadException.cpp
@@ -24,7 +24,7 @@ IceUtil::ThreadSyscallException::ice_name() const
     return _name;
 }
 
-IceUtil::Exception*
+IceUtil::ThreadSyscallException*
 IceUtil::ThreadSyscallException::ice_clone() const
 {
     return new ThreadSyscallException(*this);
@@ -49,7 +49,7 @@ IceUtil::ThreadLockedException::ice_name() const
     return _name;
 }
 
-IceUtil::Exception*
+IceUtil::ThreadLockedException*
 IceUtil::ThreadLockedException::ice_clone() const
 {
     return new ThreadLockedException(*this);
@@ -74,7 +74,7 @@ IceUtil::ThreadStartedException::ice_name() const
     return _name;
 }
 
-IceUtil::Exception*
+IceUtil::ThreadStartedException*
 IceUtil::ThreadStartedException::ice_clone() const
 {
     return new ThreadStartedException(*this);
@@ -99,7 +99,7 @@ IceUtil::ThreadNotStartedException::ice_name() const
     return _name;
 }
 
-IceUtil::Exception*
+IceUtil::ThreadNotStartedException*
 IceUtil::ThreadNotStartedException::ice_clone() const
 {
     return new ThreadNotStartedException(*this);
@@ -125,7 +125,7 @@ IceUtil::BadThreadControlException::ice_name() const
     return _name;
 }
 
-IceUtil::Exception*
+IceUtil::BadThreadControlException*
 IceUtil::BadThreadControlException::ice_clone() const
 {
     return new BadThreadControlException(*this);
@@ -159,7 +159,7 @@ IceUtil::InvalidTimeoutException::ice_print(ostream& os) const
     os << ":\ninvalid timeout: " << _timeout << " seconds";
 }
 
-IceUtil::Exception*
+IceUtil::InvalidTimeoutException*
 IceUtil::InvalidTimeoutException::ice_clone() const
 {
     return new InvalidTimeoutException(*this);
diff --git a/cpp/src/IceUtil/Unicode.cpp b/cpp/src/IceUtil/Unicode.cpp
index f7a8bd5192a..238ae2042ff 100644
--- a/cpp/src/IceUtil/Unicode.cpp
+++ b/cpp/src/IceUtil/Unicode.cpp
@@ -165,7 +165,7 @@ IceUtil::UTFConversionException::ice_print(ostream& os) const
     };
 }
 
-IceUtil::Exception*
+IceUtil::UTFConversionException*
 IceUtil::UTFConversionException::ice_clone() const
 {
     return new UTFConversionException(*this);
diff --git a/cpp/src/IceXML/Parser.cpp b/cpp/src/IceXML/Parser.cpp
index d72164c5c5c..d561cf70459 100644
--- a/cpp/src/IceXML/Parser.cpp
+++ b/cpp/src/IceXML/Parser.cpp
@@ -55,7 +55,7 @@ IceXML::ParserException::ice_print(std::ostream& out) const
     }
 }
 
-IceUtil::Exception*
+IceXML::ParserException*
 IceXML::ParserException::ice_clone() const
 {
     return new ParserException(*this);
diff --git a/cpp/src/Slice/FileTracker.cpp b/cpp/src/Slice/FileTracker.cpp
index 7dcd7e96e42..9000efb43a0 100644
--- a/cpp/src/Slice/FileTracker.cpp
+++ b/cpp/src/Slice/FileTracker.cpp
@@ -42,7 +42,7 @@ Slice::FileException::ice_print(ostream& out) const
     out << ": " << _reason;
 }
 
-IceUtil::Exception*
+Slice::FileException*
 Slice::FileException::ice_clone() const
 {
     return new FileException(*this);
diff --git a/cpp/src/slice2cpp/Gen.cpp b/cpp/src/slice2cpp/Gen.cpp
index 1d713299433..7aefe066ea3 100644
--- a/cpp/src/slice2cpp/Gen.cpp
+++ b/cpp/src/slice2cpp/Gen.cpp
@@ -825,8 +825,8 @@ Slice::Gen::TypesVisitor::visitExceptionStart(const ExceptionPtr& p)
         H << nl << "virtual void ice_print(::std::ostream&) const;";
     }
 
-    H << nl << "virtual ::Ice::Exception* ice_clone() const;";
-    C << sp << nl << "::Ice::Exception*" << nl << scoped.substr(2) << "::ice_clone() const";
+    H << nl << "virtual " << name << "* ice_clone() const;";
+    C << sp << nl << scoped.substr(2) << "*" << nl << scoped.substr(2) << "::ice_clone() const";
     C << sb;
     C << nl << "return new " << name << "(*this);";
     C << eb;
@@ -3559,27 +3559,37 @@ Slice::Gen::ObjectVisitor::visitClassDefStart(const ClassDefPtr& p)
 
     if(!p->isLocal())
     {
-        H << nl << "virtual ::Ice::ObjectPtr ice_clone() const;";
-
         C << sp << nl
 	  << (_dllExport.empty() ? "" : "ICE_DECLSPEC_EXPORT ")
 	  << "::Ice::Object* " << scope.substr(2) << "upCast(" << scoped << "* p) { return p; }";
 
-        C << sp;
-        C << nl << "::Ice::ObjectPtr";
-        C << nl << scoped.substr(2) << "::ice_clone() const";
-        C << sb;
-        if(!p->isAbstract())
-        {
-            C << nl << fixKwd(p->scope()) << p->name() << "Ptr __p = new " << scoped << "(*this);";
-            C << nl << "return __p;";
-        }
-        else
-        {
-            C << nl << "throw ::Ice::CloneNotImplementedException(__FILE__, __LINE__);";
-            C << nl << "return 0; // to avoid a warning with some compilers";
-        }
-        C << eb;
+	//
+	// It would make sense to provide a covariant ice_clone(); unfortunately many compilers
+	// (including VS2010) generate bad code for covariant types that use virtual inheritance 
+	//
+
+	if(!p->isInterface())
+	{
+	    H << nl << "virtual ::Ice::ObjectPtr ice_clone() const;";
+	   
+	    C << nl << "::Ice::ObjectPtr";
+	    C << nl << scoped.substr(2) << "::ice_clone() const";
+	    C << sb;
+	    if(!p->isAbstract())
+	    {
+		C << nl << "::Ice::Object* __p = new " << name << "(*this);";
+		C << nl << "return __p;";
+	    }
+	    else
+	    {
+		//
+		// We need this ice_clone for abstract classes derived from concrete classes
+		//
+		C << nl << "throw ::Ice::CloneNotImplementedException(__FILE__, __LINE__);";
+		C << nl << "return 0; // to avoid a warning with some compilers";
+	    }
+	    C << eb;
+	}
 
         ClassList allBases = p->allBases();
         StringList ids;