ICE-4379 - marshaling order of C# stacks

5 files changed, 95 insertions, 94 deletions

diff --git a/CHANGES b/CHANGES
index bce1eb20382..83432dd5671 100644
--- a/CHANGES
+++ b/CHANGES
@@ -163,6 +163,12 @@ Visual Studio Add-in Changes
 C# Changes
 ==========
 
+- Changed the marshaling code for sequences of user-defined types that
+  are mapped to System.Collections.Generic.Stack<T>. In previous
+  releases the elements were marshaled in bottom-to-top order; now
+  they are marshaled in top-to-bottom order. Stacks of primitve types
+  have always been marshaled in top-to-bottom orrder.
+
 - Fixed a bug that could cause an infinite loop while visiting all
   referenced assemblies of an executable.
 
diff --git a/cpp/src/Slice/CsUtil.cpp b/cpp/src/Slice/CsUtil.cpp
index e3841b72e9e..cf9597e2e91 100644
--- a/cpp/src/Slice/CsUtil.cpp
+++ b/cpp/src/Slice/CsUtil.cpp
@@ -1148,11 +1148,11 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
                         if(isStack)
                         {
                             //
-                            // If the collection is a stack, write in bottom-to-top order. Stacks
+                            // If the collection is a stack, write in top-to-bottom order. Stacks
                             // cannot contain Ice.Object.
                             //
                             out << nl << "Ice.ObjectPrx[] " << param << "_tmp = " << param << ".ToArray();";
-                            out << nl << "for(int ix__ = " << param << "_tmp.Length - 1; ix__ >= 0; --ix__)";
+                            out << nl << "for(int ix__ = 0; ix__ < " << param << "_tmp.Length; ++ix__)";
                             out << sb;
                             out << nl << stream << ".writeProxy(" << param << "_tmp[ix__]);";
                             out << eb;
@@ -1209,10 +1209,6 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
                         out << nl << "for(int ix__ = 0; ix__ < " << param << "_lenx; ++ix__)";
                         out << sb;
                         out << nl << stream << ".readObject(";
-                        if(streamingAPI)
-                        {
-                            out << "(ReadObjectCallback)";
-                        }
                         string patcherName;
                         if(isCustom)
                         {
@@ -1373,17 +1369,13 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
                             if(isStack)
                             {
                                 //
-                                // Stacks are marshaled in top-to-bottom order. We cannot call
-                                // "new Stack(type[])" because that constructor assumes the array
-                                // is in bottom-to-top order. We read the array first, then push it
-                                // in reverse order.
+                                // Stacks are marshaled in top-to-bottom order. The "Stack(type[])"
+                                // constructor assumes the array is in bottom-to-top order, so we
+                                // read the array first, then reverse it.
                                 //
                                 out << nl << typeS << "[] arr__ = " << stream << ".read" << func << "Seq();";
-                                out << nl << param << " = new " << typeToString(seq) << "(arr__.Length);";
-                                out << nl << "for(int ix__ = arr__.Length - 1; ix__ >= 0; --ix__)";
-                                out << sb;
-                                out << nl << param << ".Push(arr__[ix__]);";
-                                out << eb;
+                                out << nl << "_System.Array.Reverse(arr__);";
+                                out << nl << param << " = new " << typeToString(seq) << "(arr__);";
                             }
                             else
                             {
@@ -1489,10 +1481,6 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
             out << nl << "IceInternal." << patcherName << "Patcher<" << typeS << "> spx = new IceInternal."
                 << patcherName << "Patcher<" << typeS << ">(\"" << scoped << "\", " << param << ", ix__);";
             out << nl << stream << ".readObject(";
-            if(streamingAPI)
-            {
-                out << "(Ice.ReadObjectCallback)";
-            }
             out << "spx);";
             out << eb;
             out << eb;
@@ -1515,12 +1503,12 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
             if(isGeneric && !isList)
             {
                 //
-                // Stacks are marshaled bottom-up.
+                // Stacks are marshaled top-down.
                 //
                 if(isStack)
                 {
                     out << nl << typeS << "[] " << param << "_tmp = " << param << ".ToArray();";
-                    out << nl << "for(int ix__ = " << param << "_tmp.Length - 1; ix__ >= 0; --ix__)";
+                    out << nl << "for(int ix__ = 0; ix__ < " << param << "_tmp.Length; ++ix__)";
                 }
                 else
                 {
@@ -1589,18 +1577,21 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
             out << sb;
             out << nl << "int szx__ = " << stream << ".readAndCheckSeqSize(" 
                 << static_cast<unsigned>(type->minWireSize()) << ");";
-            out << nl << param << " = new ";
             if(isArray)
             {
-                out << toArrayAlloc(typeS + "[]", "szx__") << ";";
+                out << nl << param << " = new " << toArrayAlloc(typeS + "[]", "szx__") << ";";
             }
             else if(isCustom)
             {
-                out << "global::" << genericType << "<" << typeS << ">();";
+                out << nl << param << " = new global::" << genericType << "<" << typeS << ">();";
+            }
+            else if(isStack)
+            {
+                out << nl << typeS << "[] " << param << "__tmp = new " << toArrayAlloc(typeS + "[]", "szx__") << ";";
             }
             else if(isGeneric)
             {
-                out << "_System.Collections.Generic." << genericType << "<" << typeS << ">(";
+                out << nl << param << " = new _System.Collections.Generic." << genericType << "<" << typeS << ">(";
                 if(!isLinkedList)
                 {
                     out << "szx__";
@@ -1609,23 +1600,24 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
             }
             else
             {
-                out << fixId(seq->scoped()) << "(szx__);";
+                out << nl << param << " = new " << fixId(seq->scoped()) << "(szx__);";
             }
             out << nl << "for(int ix__ = 0; ix__ < szx__; ++ix__)";
             out << sb;
-            if(isArray)
+            if(isArray || isStack)
             {
+                string v = isArray ? param : param + "__tmp";
                 if(!isValueType(st))
                 {
-                    out << nl << param << "[ix__] = new " << typeS << "();";
+                    out << nl << v << "[ix__] = new " << typeS << "();";
                 }
                 if(streamingAPI)
                 {
-                    out << nl << param << "[ix__].ice_read(" << stream << ");";
+                    out << nl << v << "[ix__].ice_read(" << stream << ");";
                 }
                 else
                 {
-                    out << nl << param << "[ix__].read__(" << stream << ");";
+                    out << nl << v << "[ix__].read__(" << stream << ");";
                 }
             }
             else
@@ -1642,6 +1634,12 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
                 out << nl << param << "." << addMethod << "(val__);";
             }
             out << eb;
+            if(isStack)
+            {
+                out << nl << "_System.Array.Reverse(" << param << "__tmp);";
+                out << nl << param << " = new _System.Collections.Generic." << genericType << "<" << typeS << ">("
+                    << param << "__tmp);";
+            }
             out << eb;
         }
         return;
@@ -1650,7 +1648,6 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
     EnumPtr en = EnumPtr::dynamicCast(type);
     if(en)
     {
-        size_t sz = en->getEnumerators().size();
         if(marshal)
         {
             out << nl << "if(" << param << " == null)";
@@ -1663,14 +1660,14 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
             if(isGeneric && !isList)
             {
                 //
-                // Stacks are marshaled bottom-up.
+                // Stacks are marshaled top-down.
                 //
                 if(isStack)
                 {
                     out << nl << typeS << "[] " << param << "_tmp = " << param << ".ToArray();";
-                    out << nl << "for(int ix__ = " << param << "_tmp.Length - 1; ix__ >= 0; --ix__)";
+                    out << nl << "for(int ix__ = 0; ix__ < " << param << "_tmp.Length; ++ix__)";
                     out << sb;
-                    out << nl << stream << ".writeEnum((int)" << param << "_tmp[ix__], " << sz << ");";
+                    out << nl << stream << ".writeEnum((int)" << param << "_tmp[ix__], " << en->maxValue() << ");";
                     out << eb;
                 }
                 else
@@ -1679,7 +1676,7 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
                         << "> e__ = " << param << ".GetEnumerator();";
                     out << nl << "while(e__.MoveNext())";
                     out << sb;
-                    out << nl << stream << ".writeEnum((int)e__.Current, " << sz << ");";
+                    out << nl << stream << ".writeEnum((int)e__.Current, " << en->maxValue() << ");";
                     out << eb;
                 }
             }
@@ -1687,7 +1684,7 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
             {
                 out << nl << "for(int ix__ = 0; ix__ < " << param << '.' << limitID << "; ++ix__)";
                 out << sb;
-                out << nl << stream << ".writeEnum((int)" << param << "[ix__], " << sz << ");";
+                out << nl << stream << ".writeEnum((int)" << param << "[ix__], " << en->maxValue() << ");";
                 out << eb;
             }
             out << eb;
@@ -1697,18 +1694,21 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
             out << sb;
             out << nl << "int szx__ = " << stream << ".readAndCheckSeqSize(" << 
                 static_cast<unsigned>(type->minWireSize()) << ");";
-            out << nl << param << " = new ";
             if(isArray)
             {
-                out << toArrayAlloc(typeS + "[]", "szx__") << ";";
+                out << nl << param << " = new " << toArrayAlloc(typeS + "[]", "szx__") << ";";
             }
             else if(isCustom)
             {
-                out << "global::" << genericType << "<" << typeS << ">();";
+                out << nl << param << " = new global::" << genericType << "<" << typeS << ">();";
+            }
+            else if(isStack)
+            {
+                out << nl << typeS << "[] " << param << "__tmp = new " << toArrayAlloc(typeS + "[]", "szx__") << ";";
             }
             else if(isGeneric)
             {
-                out << "_System.Collections.Generic." << genericType << "<" << typeS << ">(";
+                out << nl << param << " = new _System.Collections.Generic." << genericType << "<" << typeS << ">(";
                 if(!isLinkedList)
                 {
                     out << "szx__";
@@ -1717,19 +1717,27 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
             }
             else
             {
-                out << fixId(seq->scoped()) << "(szx__);";
+                out << nl << param << " = new " << fixId(seq->scoped()) << "(szx__);";
             }
             out << nl << "for(int ix__ = 0; ix__ < szx__; ++ix__)";
             out << sb;
-            if(isArray)
+            if(isArray || isStack)
             {
-                out << nl << param << "[ix__] = (" << typeS << ')' << stream << ".readEnum(" << sz << ");";
+                string v = isArray ? param : param + "__tmp";
+                out << nl << v << "[ix__] = (" << typeS << ')' << stream << ".readEnum(" << en->maxValue() << ");";
             }
             else
             {
-                out << nl << param << "." << addMethod << "((" << typeS << ')' << stream << ".readEnum(" << sz << "));";
+                out << nl << param << "." << addMethod << "((" << typeS << ')' << stream << ".readEnum("
+                    << en->maxValue() << "));";
             }
             out << eb;
+            if(isStack)
+            {
+                out << nl << "_System.Array.Reverse(" << param << "__tmp);";
+                out << nl << param << " = new _System.Collections.Generic." << genericType << "<" << typeS << ">("
+                    << param << "__tmp);";
+            }
             out << eb;
         }
         return;
@@ -1763,12 +1771,12 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
         if(isGeneric && !isList)
         {
             //
-            // Stacks are marshaled bottom-up.
+            // Stacks are marshaled top-down.
             //
             if(isStack)
             {
                 out << nl << typeS << "[] " << param << "_tmp = " << param << ".ToArray();";
-                out << nl << "for(int ix__ = " << param << "_tmp.Length - 1; ix__ >= 0; --ix__)";
+                out << nl << "for(int ix__ = 0; ix__ < " << param << "_tmp.Length; ++ix__)";
                 out << sb;
                 out << nl << helperName << '.' << func << '(' << stream << ", " << param << "_tmp[ix__]);";
                 out << eb;
@@ -1802,34 +1810,44 @@ Slice::CsGenerator::writeSequenceMarshalUnmarshalCode(Output& out,
         out << sb;
         out << nl << "int szx__ = " << stream << ".readAndCheckSeqSize("
             << static_cast<unsigned>(type->minWireSize()) << ");";
-        out << nl << param << " = new ";
         if(isArray)
         {
-            out << toArrayAlloc(typeS + "[]", "szx__") << ";";
+            out << nl << param << " = new " << toArrayAlloc(typeS + "[]", "szx__") << ";";
         }
         else if(isCustom)
         {
-            out << "global::" << genericType << "<" << typeS << ">();";
+            out << nl << param << " = new global::" << genericType << "<" << typeS << ">();";
+        }
+        else if(isStack)
+        {
+            out << nl << typeS << "[] " << param << "__tmp = new " << toArrayAlloc(typeS + "[]", "szx__") << ";";
         }
         else if(isGeneric)
         {
-            out << "_System.Collections.Generic." << genericType << "<" << typeS << ">();";
+            out << nl << param << " = new _System.Collections.Generic." << genericType << "<" << typeS << ">();";
         }
         else
         {
-            out << fixId(seq->scoped()) << "(szx__);";
+            out << nl << param << " = new " << fixId(seq->scoped()) << "(szx__);";
         }
         out << nl << "for(int ix__ = 0; ix__ < szx__; ++ix__)";
         out << sb;
-        if(isArray)
+        if(isArray || isStack)
         {
-            out << nl << param << "[ix__] = " << helperName << '.' << func << '(' << stream << ");";
+            string v = isArray ? param : param + "__tmp";
+            out << nl << v << "[ix__] = " << helperName << '.' << func << '(' << stream << ");";
         }
         else
         {
             out << nl << param << "." << addMethod << "(" << helperName << '.' << func << '(' << stream << "));";
         }
         out << eb;
+        if(isStack)
+        {
+            out << nl << "_System.Array.Reverse(" << param << "__tmp);";
+            out << nl << param << " = new _System.Collections.Generic." << genericType << "<" << typeS << ">("
+                << param << "__tmp);";
+        }
         out << eb;
     }
 
diff --git a/cs/src/Ice/BasicStream.cs b/cs/src/Ice/BasicStream.cs
index f21dbe7aa1a..4c6f636fa04 100644
--- a/cs/src/Ice/BasicStream.cs
+++ b/cs/src/Ice/BasicStream.cs
@@ -1063,11 +1063,8 @@ namespace IceInternal
             // because the stack is marshaled in top-to-bottom order.
             //
             byte[] array = readByteSeq();
-            l = new Stack<byte>(array.Length);
-            for(int i = array.Length - 1; i >= 0; --i)
-            {
-                l.Push(array[i]);
-            }
+            Array.Reverse(array);
+            l = new Stack<byte>(array);
         }
 
         public Ice.Optional<byte[]> readByteSeq(int tag)
@@ -1337,11 +1334,8 @@ namespace IceInternal
             // because the stack is marshaled in top-to-bottom order.
             //
             bool[] array = readBoolSeq();
-            l = new Stack<bool>(array.Length);
-            for(int i = array.Length - 1; i >= 0; --i)
-            {
-                l.Push(array[i]);
-            }
+            Array.Reverse(array);
+            l = new Stack<bool>(array);
         }
 
         public Ice.Optional<bool[]> readBoolSeq(int tag)
@@ -1586,11 +1580,8 @@ namespace IceInternal
             // because the stack is marshaled in top-to-bottom order.
             //
             short[] array = readShortSeq();
-            l = new Stack<short>(array.Length);
-            for(int i = array.Length - 1; i >= 0; --i)
-            {
-                l.Push(array[i]);
-            }
+            Array.Reverse(array);
+            l = new Stack<short>(array);
         }
 
         public Ice.Optional<short[]> readShortSeq(int tag)
@@ -1862,11 +1853,8 @@ namespace IceInternal
             // because the stack is marshaled in top-to-bottom order.
             //
             int[] array = readIntSeq();
-            l = new Stack<int>(array.Length);
-            for(int i = array.Length - 1; i >= 0; --i)
-            {
-                l.Push(array[i]);
-            }
+            Array.Reverse(array);
+            l = new Stack<int>(array);
         }
 
         public Ice.Optional<int[]> readIntSeq(int tag)
@@ -2133,11 +2121,8 @@ namespace IceInternal
             // because the stack is marshaled in top-to-bottom order.
             //
             long[] array = readLongSeq();
-            l = new Stack<long>(array.Length);
-            for(int i = array.Length - 1; i >= 0; --i)
-            {
-                l.Push(array[i]);
-            }
+            Array.Reverse(array);
+            l = new Stack<long>(array);
         }
 
         public Ice.Optional<long[]> readLongSeq(int tag)
@@ -2404,11 +2389,8 @@ namespace IceInternal
             // because the stack is marshaled in top-to-bottom order.
             //
             float[] array = readFloatSeq();
-            l = new Stack<float>(array.Length);
-            for(int i = array.Length - 1; i >= 0; --i)
-            {
-                l.Push(array[i]);
-            }
+            Array.Reverse(array);
+            l = new Stack<float>(array);
         }
 
         public Ice.Optional<float[]> readFloatSeq(int tag)
@@ -2675,11 +2657,8 @@ namespace IceInternal
             // because the stack is marshaled in top-to-bottom order.
             //
             double[] array = readDoubleSeq();
-            l = new Stack<double>(array.Length);
-            for(int i = array.Length - 1; i >= 0; --i)
-            {
-                l.Push(array[i]);
-            }
+            Array.Reverse(array);
+            l = new Stack<double>(array);
         }
 
         public Ice.Optional<double[]> readDoubleSeq(int tag)
@@ -2933,11 +2912,8 @@ namespace IceInternal
             // because the stack is marshaled in top-to-bottom order.
             //
             string[] array = readStringSeq();
-            l = new Stack<string>(array.Length);
-            for(int i = array.Length - 1; i >= 0; --i)
-            {
-                l.Push(array[i]);
-            }
+            Array.Reverse(array);
+            l = new Stack<string>(array);
         }
 
         public Ice.Optional<string[]> readStringSeq(int tag)
diff --git a/cs/test/Ice/seqMapping/Makefile b/cs/test/Ice/seqMapping/Makefile
index 0651d35e703..9affa1ab7cb 100644
--- a/cs/test/Ice/seqMapping/Makefile
+++ b/cs/test/Ice/seqMapping/Makefile
@@ -28,7 +28,7 @@ include $(top_srcdir)/config/Make.rules.cs
 
 MCSFLAGS	:= $(MCSFLAGS) -target:exe
 
-SLICE2CSFLAGS	:= $(SLICE2CSFLAGS) --ice -I. -I$(slicedir)
+SLICE2CSFLAGS	:= $(SLICE2CSFLAGS) --stream
 
 client.exe: $(C_SRCS) $(GEN_SRCS) Serializable.dll
 	$(MCS) $(MCSFLAGS) -out:$@ $(call ref,Ice) -r:Serializable.dll $(C_SRCS) $(GEN_SRCS)
diff --git a/cs/test/Ice/seqMapping/Makefile.mak b/cs/test/Ice/seqMapping/Makefile.mak
index 773238e38eb..c30c9fb5b9e 100644
--- a/cs/test/Ice/seqMapping/Makefile.mak
+++ b/cs/test/Ice/seqMapping/Makefile.mak
@@ -27,6 +27,7 @@ GDIR		= generated
 

 MCSFLAGS	= $(MCSFLAGS) -target:exe

 

+SLICE2CSFLAGS	= $(SLICE2CSFLAGS) --stream

 

 client.exe: $(C_SRCS) $(GEN_SRCS) Serializable.dll

 	$(MCS) $(MCSFLAGS) -out:$@ -r:"$(refdir)\Ice.dll" -r:Serializable.dll $(C_SRCS) $(GEN_SRCS)