Fixed bug in CSharp Stack sequence mapping

5 files changed, 200 insertions, 88 deletions

diff --git a/CHANGELOG-3.6.md b/CHANGELOG-3.6.md
index b5495057098..f770a4b31c6 100644
--- a/CHANGELOG-3.6.md
+++ b/CHANGELOG-3.6.md
@@ -68,6 +68,14 @@ These are the changes since Ice 3.6.4.
 - Fixed a bug in Ice.Long toNumber implementation where negative integers
   smaller than -(2^52 - 1) where not correctly handle.
 
+## CSharp Changes
+
+- Fixed a bug that affect Stack sequence mapping, when using the Stack mapping
+with a element of type Object* items where unmarshal in reverse order.
+
+- Fixed a bug where metadata was not correctly ignored and can result in bogus
+code being generated if applying invalid metadata directives. 
+
 # Changes in Ice 3.6.3
 
 These are the changes since Ice 3.6.2.
diff --git a/cpp/src/slice2cs/CsUtil.cpp b/cpp/src/slice2cs/CsUtil.cpp
index 85e121c1967..251b910f473 100644
--- a/cpp/src/slice2cs/CsUtil.cpp
+++ b/cpp/src/slice2cs/CsUtil.cpp
@@ -2355,6 +2355,7 @@ Slice::CsGenerator::MetaDataVisitor::visitUnitStart(const UnitPtr& p)
         DefinitionContextPtr dc = p->findDefinitionContext(file);
         assert(dc);
         StringList globalMetaData = dc->getMetaData();
+        StringList newGlobalMetaData;
 
         static const string csPrefix = "cs:";
         static const string clrPrefix = "clr:";
@@ -2369,22 +2370,19 @@ Slice::CsGenerator::MetaDataVisitor::visitUnitStart(const UnitPtr& p)
                 s.replace(0, clrPrefix.size(), csPrefix);
             }
 
-            if(_history.count(s) == 0)
+            if(s.find(csPrefix) == 0)
             {
-                if(s.find(csPrefix) == 0)
+                static const string csAttributePrefix = csPrefix + "attribute:";
+                if(s.find(csAttributePrefix) != 0 || s.size() == csAttributePrefix.size())
                 {
-                    static const string csAttributePrefix = csPrefix + "attribute:";
-                    if(s.find(csAttributePrefix) == 0 && s.size() > csAttributePrefix.size())
-                    {
-                        continue;
-                    }
                     emitWarning(file, -1, "ignoring invalid global metadata `" + oldS + "'");
-                    _history.insert(s);
+                    continue;
                 }
             }
+            newGlobalMetaData.push_back(oldS);
         }
 
-        dc->setMetaData(globalMetaData);
+        dc->setMetaData(newGlobalMetaData);
     }
     return true;
 }
@@ -2497,6 +2495,7 @@ Slice::CsGenerator::MetaDataVisitor::validate(const ContainedPtr& cont)
     const string msg = "ignoring invalid metadata";
 
     StringList localMetaData = cont->getMetaData();
+    StringList newLocalMetaData;
 
     for(StringList::iterator p = localMetaData.begin(); p != localMetaData.end(); ++p)
     {
@@ -2511,113 +2510,125 @@ Slice::CsGenerator::MetaDataVisitor::validate(const ContainedPtr& cont)
             s.replace(0, clrPrefix.size(), csPrefix);
         }
 
-        if(_history.count(s) == 0)
+
+        if(s.find(csPrefix) == 0)
         {
-            if(s.find(csPrefix) == 0)
+            SequencePtr seq = SequencePtr::dynamicCast(cont);
+            if(seq)
             {
-                SequencePtr seq = SequencePtr::dynamicCast(cont);
-                if(seq)
+                static const string csGenericPrefix = csPrefix + "generic:";
+                if(s.find(csGenericPrefix) == 0)
                 {
-                    static const string csGenericPrefix = csPrefix + "generic:";
-                    if(s.find(csGenericPrefix) == 0)
+                    string type = s.substr(csGenericPrefix.size());
+                    if(type == "LinkedList" || type == "Queue" || type == "Stack")
                     {
-                        string type = s.substr(csGenericPrefix.size());
-                        if(type == "LinkedList" || type == "Queue" || type == "Stack")
-                        {
-                            if(!isClassType(seq->type()))
-                            {
-                                continue;
-                            }
-                        }
-                        else if(!type.empty())
-                        {
-                            continue; // Custom type or List<T>
-                        }
-                    }
-                    static const string csSerializablePrefix = csPrefix + "serializable:";
-                    if(s.find(csSerializablePrefix) == 0)
-                    {
-                        string meta;
-                        if(cont->findMetaData(csPrefix + "generic:", meta))
-                        {
-                            emitWarning(cont->file(), cont->line(), msg + " `" + meta + "':\n" +
-                                        "serialization can only be used with the array mapping for byte sequences");
-                        }
-                        string type = s.substr(csSerializablePrefix.size());
-                        BuiltinPtr builtin = BuiltinPtr::dynamicCast(seq->type());
-                        if(!type.empty() && builtin && builtin->kind() == Builtin::KindByte)
+                        if(!isClassType(seq->type()))
                         {
+                            newLocalMetaData.push_back(s);
                             continue;
                         }
                     }
-                }
-                else if(StructPtr::dynamicCast(cont))
-                {
-                    if(s.substr(csPrefix.size()) == "class")
-                    {
-                        continue;
-                    }
-                    if(s.substr(csPrefix.size()) == "property")
+                    else if(!type.empty())
                     {
-                        continue;
-                    }
-                    static const string csImplementsPrefix = csPrefix + "implements:";
-                    if(s.find(csImplementsPrefix) == 0)
-                    {
-                        continue;
+                        newLocalMetaData.push_back(s);
+                        continue; // Custom type or List<T>
                     }
                 }
-                else if(ClassDefPtr::dynamicCast(cont))
+                static const string csSerializablePrefix = csPrefix + "serializable:";
+                if(s.find(csSerializablePrefix) == 0)
                 {
-                    if(s.substr(csPrefix.size()) == "property")
+                    string meta;
+                    if(cont->findMetaData(csPrefix + "generic:", meta))
                     {
+                        emitWarning(cont->file(), cont->line(), msg + " `" + meta + "':\n" +
+                                    "serialization can only be used with the array mapping for byte sequences");
                         continue;
                     }
-                    static const string csImplementsPrefix = csPrefix + "implements:";
-                    if(s.find(csImplementsPrefix) == 0)
+                    string type = s.substr(csSerializablePrefix.size());
+                    BuiltinPtr builtin = BuiltinPtr::dynamicCast(seq->type());
+                    if(!type.empty() && builtin && builtin->kind() == Builtin::KindByte)
                     {
+                        newLocalMetaData.push_back(s);
                         continue;
                     }
                 }
-                else if(DictionaryPtr::dynamicCast(cont))
+            }
+            else if(StructPtr::dynamicCast(cont))
+            {
+                if(s.substr(csPrefix.size()) == "class")
                 {
-                    static const string csGenericPrefix = csPrefix + "generic:";
-                    if(s.find(csGenericPrefix) == 0)
-                    {
-                        string type = s.substr(csGenericPrefix.size());
-                        if(type == "SortedDictionary" ||  type == "SortedList")
-                        {
-                            continue;
-                        }
-                    }
+                    newLocalMetaData.push_back(s);
+                    continue;
                 }
-
-                static const string csAttributePrefix = csPrefix + "attribute:";
-                static const string csTie = csPrefix + "tie";
-                if(s.find(csAttributePrefix) == 0 && s.size() > csAttributePrefix.size())
+                if(s.substr(csPrefix.size()) == "property")
                 {
+                    newLocalMetaData.push_back(s);
                     continue;
                 }
-                else if(s.find(csTie) == 0 && s.size() == csTie.size())
+                static const string csImplementsPrefix = csPrefix + "implements:";
+                if(s.find(csImplementsPrefix) == 0)
                 {
+                    newLocalMetaData.push_back(s);
                     continue;
                 }
-
-                emitWarning(cont->file(), cont->line(), msg + " `" + oldS + "'");
-                _history.insert(s);
             }
-            else if(s == "delegate")
+            else if(ClassDefPtr::dynamicCast(cont))
             {
-                ClassDefPtr cl = ClassDefPtr::dynamicCast(cont);
-                if(cl && cl->isDelegate())
+                if(s.substr(csPrefix.size()) == "property")
+                {
+                    newLocalMetaData.push_back(s);
+                    continue;
+                }
+                static const string csImplementsPrefix = csPrefix + "implements:";
+                if(s.find(csImplementsPrefix) == 0)
                 {
+                    newLocalMetaData.push_back(s);
                     continue;
                 }
-                emitWarning(cont->file(), cont->line(), msg + " `" + s + "'");
-                _history.insert(s);
             }
+            else if(DictionaryPtr::dynamicCast(cont))
+            {
+                static const string csGenericPrefix = csPrefix + "generic:";
+                if(s.find(csGenericPrefix) == 0)
+                {
+                    string type = s.substr(csGenericPrefix.size());
+                    if(type == "SortedDictionary" ||  type == "SortedList")
+                    {
+                        newLocalMetaData.push_back(s);
+                        continue;
+                    }
+                }
+            }
+
+            static const string csAttributePrefix = csPrefix + "attribute:";
+            static const string csTie = csPrefix + "tie";
+            if(s.find(csAttributePrefix) == 0 && s.size() > csAttributePrefix.size())
+            {
+                newLocalMetaData.push_back(s);
+                continue;
+            }
+            else if(s.find(csTie) == 0 && s.size() == csTie.size())
+            {
+                newLocalMetaData.push_back(s);
+                continue;
+            }
+
+            emitWarning(cont->file(), cont->line(), msg + " `" + oldS + "'");
+            continue;
+        }
+        else if(s == "delegate")
+        {
+            ClassDefPtr cl = ClassDefPtr::dynamicCast(cont);
+            if(cl && cl->isDelegate())
+            {
+                newLocalMetaData.push_back(s);
+                continue;
+            }
+            emitWarning(cont->file(), cont->line(), msg + " `" + s + "'");
+            continue;
         }
+        newLocalMetaData.push_back(s);
     }
 
-    cont->setMetaData(localMetaData);
+    cont->setMetaData(newLocalMetaData);
 }
diff --git a/cpp/src/slice2cs/CsUtil.h b/cpp/src/slice2cs/CsUtil.h
index b25c6a3dd2f..16ebd8a3c54 100644
--- a/cpp/src/slice2cs/CsUtil.h
+++ b/cpp/src/slice2cs/CsUtil.h
@@ -88,7 +88,6 @@ private:
         void validate(const ContainedPtr&);
 
         std::string _fileName;
-        StringSet _history;
     };
 };
 
diff --git a/csharp/test/Ice/stream/AllTests.cs b/csharp/test/Ice/stream/AllTests.cs
index a1ae01f2f10..199562f7297 100644
--- a/csharp/test/Ice/stream/AllTests.cs
+++ b/csharp/test/Ice/stream/AllTests.cs
@@ -136,6 +136,11 @@ public class AllTests : TestCommon.AllTests
         MyClassFactoryWrapper factoryWrapper = new MyClassFactoryWrapper();
 
         communicator.getValueFactoryManager().add(factoryWrapper.create, MyClass.ice_staticId());
+        communicator.getValueFactoryManager().add((id) =>
+            {
+                return new Ice.InterfaceByValue("::Test::MyInterface");
+            },
+            "::Test::MyInterface");
 
         Ice.InputStream inS;
         Ice.OutputStream outS;
@@ -506,7 +511,7 @@ public class AllTests : TestCommon.AllTests
         }
 
         var smallStructArray = new SmallStruct[3];
-        for (int i = 0; i < smallStructArray.Length; ++i)
+        for(int i = 0; i < smallStructArray.Length; ++i)
         {
             smallStructArray[i] = new SmallStruct();
             smallStructArray[i].bo = true;
@@ -522,7 +527,7 @@ public class AllTests : TestCommon.AllTests
         }
 
         var myClassArray = new MyClass[4];
-        for (int i = 0; i < myClassArray.Length; ++i)
+        for(int i = 0; i < myClassArray.Length; ++i)
         {
             myClassArray[i] = new MyClass();
             myClassArray[i].c = myClassArray[i];
@@ -543,6 +548,13 @@ public class AllTests : TestCommon.AllTests
             myClassArray[i].d["hi"] = myClassArray[i];
         }
 
+        var myInterfaceArray = new Ice.Value[4];
+        for(int i = 0; i < myInterfaceArray.Length; ++i)
+        {
+            myInterfaceArray[i] = new Ice.InterfaceByValue("::Test::MyInterface");
+        }
+
+
         {
             outS = new Ice.OutputStream(communicator);
             MyClassSHelper.write(outS, myClassArray);
@@ -552,7 +564,7 @@ public class AllTests : TestCommon.AllTests
             var arr2 = MyClassSHelper.read(inS);
             inS.readPendingValues();
             test(arr2.Length == myClassArray.Length);
-            for (int i = 0; i < arr2.Length; ++i)
+            for(int i = 0; i < arr2.Length; ++i)
             {
                 test(arr2[i] != null);
                 test(arr2[i].c == arr2[i]);
@@ -584,6 +596,27 @@ public class AllTests : TestCommon.AllTests
 
         {
             outS = new Ice.OutputStream(communicator);
+            MyInterfaceSHelper.write(outS, myInterfaceArray);
+            outS.writePendingValues();
+            var data = outS.finished();
+            inS = new Ice.InputStream(communicator, data);
+            var arr2 = MyInterfaceSHelper.read(inS);
+            inS.readPendingValues();
+            test(arr2.Length == myInterfaceArray.Length);
+            Ice.Value[][] arrS = { myInterfaceArray, new Ice.Value[0], myInterfaceArray };
+            outS = new Ice.OutputStream(communicator);
+            MyInterfaceSSHelper.write(outS, arrS);
+            data = outS.finished();
+            inS = new Ice.InputStream(communicator, data);
+            var arr2S = MyInterfaceSSHelper.read(inS);
+            test(arr2S.Length == arrS.Length);
+            test(arr2S[0].Length == arrS[0].Length);
+            test(arr2S[1].Length == arrS[1].Length);
+            test(arr2S[2].Length == arrS[2].Length);
+        }
+
+        {
+            outS = new Ice.OutputStream(communicator);
             var obj = new MyClass();
             obj.s = new SmallStruct();
             obj.s.e = MyEnum.enum2;
@@ -769,7 +802,7 @@ public class AllTests : TestCommon.AllTests
             inS = new Ice.InputStream(communicator, data);
             var l2 = SmallStructListHelper.read(inS);
             test(l2.Count == l.Count);
-            for (int i = 0; i < l2.Count; ++i)
+            for(int i = 0; i < l2.Count; ++i)
             {
                 test(l2[i].Equals(smallStructArray[i]));
             }
@@ -785,7 +818,7 @@ public class AllTests : TestCommon.AllTests
             var l2 = MyClassListHelper.read(inS);
             inS.readPendingValues();
             test(l2.Count == l.Count);
-            for (int i = 0; i < l2.Count; ++i)
+            for(int i = 0; i < l2.Count; ++i)
             {
                 test(l2[i] != null);
                 test(l2[i].c == l2[i]);
@@ -818,6 +851,19 @@ public class AllTests : TestCommon.AllTests
         }
 
         {
+            var arr = new MyInterfacePrx[2];
+            arr[0] = MyInterfacePrxHelper.uncheckedCast(communicator.stringToProxy("zero"));
+            arr[1] = MyInterfacePrxHelper.uncheckedCast(communicator.stringToProxy("one"));
+            outS = new Ice.OutputStream(communicator);
+            var l = new List<MyInterfacePrx>(arr);
+            MyInterfaceProxyListHelper.write(outS, l);
+            byte[] data = outS.finished();
+            inS = new Ice.InputStream(communicator, data);
+            var l2 = MyInterfaceProxyListHelper.read(inS);
+            test(Compare(l2, l));
+        }
+
+        {
             short[] arr = { 0x01, 0x11, 0x12, 0x22 };
             outS = new Ice.OutputStream(communicator);
             var l = new LinkedList<short>(arr);
@@ -918,6 +964,19 @@ public class AllTests : TestCommon.AllTests
         }
 
         {
+            var arr = new MyInterfacePrx[2];
+            arr[0] = MyInterfacePrxHelper.uncheckedCast(communicator.stringToProxy("zero"));
+            arr[1] = MyInterfacePrxHelper.uncheckedCast(communicator.stringToProxy("one"));
+            outS = new Ice.OutputStream(communicator);
+            var l = new Stack<MyInterfacePrx>(arr);
+            MyInterfaceProxyStackHelper.write(outS, l);
+            var data = outS.finished();
+            inS = new Ice.InputStream(communicator, data);
+            var l2 = MyInterfaceProxyStackHelper.read(inS);
+            test(Compare(l2, l));
+        }
+
+        {
             double[] arr = { 1, 2, 3, 4 };
             outS = new Ice.OutputStream(communicator);
             var l = new Queue<double>(arr);
diff --git a/csharp/test/Ice/stream/Test.ice b/csharp/test/Ice/stream/Test.ice
index 6b985e003cf..25e1d90c366 100644
--- a/csharp/test/Ice/stream/Test.ice
+++ b/csharp/test/Ice/stream/Test.ice
@@ -22,6 +22,7 @@ enum MyEnum
 };
 
 class MyClass;
+interface MyInterface;
 
 struct SmallStruct
 {
@@ -47,6 +48,7 @@ class OptionalClass
 
 sequence<MyEnum> MyEnumS;
 sequence<MyClass> MyClassS;
+sequence<MyInterface> MyInterfaceS;
 
 sequence<Ice::BoolSeq> BoolSS;
 sequence<Ice::ByteSeq> ByteSS;
@@ -58,6 +60,7 @@ sequence<Ice::DoubleSeq> DoubleSS;
 sequence<Ice::StringSeq> StringSS;
 sequence<MyEnumS> MyEnumSS;
 sequence<MyClassS> MyClassSS;
+sequence<MyInterfaceS> MyInterfaceSS;
 
 dictionary<byte, bool> ByteBoolD;
 dictionary<short, int> ShortIntD;
@@ -77,6 +80,8 @@ sequence<SmallStruct> SmallStructList;
 sequence<MyClass> MyClassList;
 ["clr:generic:List"]
 sequence<MyClass*> MyClassProxyList;
+["clr:generic:List"]
+sequence<MyInterface*> MyInterfaceProxyList;
 
 ["clr:generic:LinkedList"]
 sequence<short> ShortLinkedList;
@@ -95,6 +100,32 @@ sequence<float> FloatStack;
 sequence<SmallStruct> SmallStructStack;
 ["clr:generic:Stack"]
 sequence<MyClass*> MyClassProxyStack;
+["clr:generic:Stack"]
+sequence<MyInterface*> MyInterfaceProxyStack;
+
+//
+// This will produce a warning and use the default 
+// sequence mapping. The generic:Stack metadata cannot be use
+// with object sequences.
+//
+["clr:generic:Stack"]
+sequence<Object> ObjectStack;
+
+//
+// This will produce a warning and use the default 
+// sequence mapping. The generic:Stack metadata cannot be use
+// with object sequences.
+//
+["clr:generic:Stack"]
+sequence<MyClass> MyClassStack;
+
+//
+// This will produce a warning and use the default 
+// sequence mapping. The generic:Stack metadata cannot be use
+// with object sequences.
+//
+["clr:generic:Stack"]
+sequence<MyInterface> MyInterfaceStack;
 
 ["clr:generic:Queue"]
 sequence<double> DoubleQueue;
@@ -139,4 +170,8 @@ exception MyException
     MyClass c;
 };
 
+interface MyInterface
+{
+};
+
 };