Fix Enumerator.Current handling in SortedDictionary<,>

* System.Collections.Generic/RBTree.cs (NodeEnumerator.check_current):
 New helper.
(NodeEnumerator.Current): Don't check invariants.
* SortedDictionary.cs (Enumerator.Current): Likewise.
(ValueCollection.Enumerator.Current): Likewise.
(KeyCollection.Enumerator.Current): Likewise.

* Test/System.Collections.Generic/SortedDictionaryTest.cs
(Enumerator_Current, KeyEnumerator_Current, ValueEnumerator_Current): New tests.

svn path=/trunk/mcs/; revision=139115

diff --git a/mcs/class/System/System.Collections.Generic/ChangeLog b/mcs/class/System/System.Collections.Generic/ChangeLog
index 0bfc5fbdfd1b2e1e6cf34b6a440e5bddfd64c726..ba7608dda775d76999d56d34f30a70ee7cbd76f9 100644 (file)
--- a/mcs/class/System/System.Collections.Generic/ChangeLog
+++ b/mcs/class/System/System.Collections.Generic/ChangeLog
@@ -1,3 +1,11 @@
+2009-07-31  Raja R Harinath  <harinath@hurrynot.org>
+
+       * RBTree.cs (NodeEnumerator.check_current): New helper.
+       (NodeEnumerator.Current): Don't check invariants.
+       * SortedDictionary.cs (Enumerator.Current): Likewise.
+       (ValueCollection.Enumerator.Current): Likewise.
+       (KeyCollection.Enumerator.Current): Likewise.
+
 2009-07-26  Miguel de Icaza  <miguel@novell.com>
 
        * Stack.cs: Check arguments.

diff --git a/mcs/class/System/System.Collections.Generic/RBTree.cs b/mcs/class/System/System.Collections.Generic/RBTree.cs
index bf4b64f616ec61d58a9718ae43068f3e45127d75..6325f32ff2e75df60cb886c22ff08fa1d2d33d24 100644 (file)
--- a/mcs/class/System/System.Collections.Generic/RBTree.cs
+++ b/mcs/class/System/System.Collections.Generic/RBTree.cs
@@ -591,22 +591,19 @@ namespace System.Collections.Generic
 
                        public void Reset ()
                        {
-                               if (tree == null)
-                                       throw new ObjectDisposedException ("enumerator");
+                               check_version ();
                                pennants = null;
                        }
 
                        public Node Current {
-                               get {
-                                       check_version ();
-                                       if (pennants == null)
-                                               throw new InvalidOperationException ("state invalid before the first MoveNext()");
-                                       return pennants.Peek ();
-                               }
+                               get { return pennants.Peek (); }
                        }
 
                        object IEnumerator.Current {
-                               get { return Current; }
+                               get {
+                                       check_current ();
+                                       return Current;
+                               }
                        }
 
                        public bool MoveNext ()
@@ -644,6 +641,13 @@ namespace System.Collections.Generic
                                if (version != tree.version)
                                        throw new InvalidOperationException ("tree modified");
                        }
+
+                       internal void check_current ()
+                       {
+                               check_version ();
+                               if (pennants == null)
+                                       throw new InvalidOperationException ("state invalid before the first MoveNext()");
+                       }
                }
        }
 }

diff --git a/mcs/class/System/System.Collections.Generic/SortedDictionary.cs b/mcs/class/System/System.Collections.Generic/SortedDictionary.cs
index 3f75cf24d9e4a1b94eb9c720f40b650854a0d397..95965c9d5635149fe186525f6f843107d61675f8 100644 (file)
--- a/mcs/class/System/System.Collections.Generic/SortedDictionary.cs
+++ b/mcs/class/System/System.Collections.Generic/SortedDictionary.cs
@@ -469,18 +469,24 @@ namespace System.Collections.Generic
                        {
                                RBTree.NodeEnumerator host;
 
+                               TValue current;
+
                                internal Enumerator (SortedDictionary<TKey,TValue> dic)
+                                       : this ()
                                {
                                        host = dic.tree.GetEnumerator ();
                                }
 
                                public TValue Current {
-                                       get { return ((Node) host.Current).value; }
+                                       get { return current; }
                                }
 
                                public bool MoveNext ()
                                {
-                                       return host.MoveNext ();
+                                       if (!host.MoveNext ())
+                                               return false;
+                                       current = ((Node) host.Current).value;
+                                       return true;
                                }
 
                                public void Dispose ()
@@ -489,7 +495,10 @@ namespace System.Collections.Generic
                                }
 
                                object IEnumerator.Current {
-                                       get { return Current; }
+                                       get {
+                                               host.check_current ();
+                                               return current;
+                                       }
                                }
 
                                void IEnumerator.Reset ()
@@ -593,18 +602,24 @@ namespace System.Collections.Generic
                        {
                                RBTree.NodeEnumerator host;
 
+                               TKey current;
+
                                internal Enumerator (SortedDictionary<TKey,TValue> dic)
+                                       : this ()
                                {
                                        host = dic.tree.GetEnumerator ();
                                }
 
                                public TKey Current {
-                                       get { return ((Node) host.Current).key; }
+                                       get { return current; }
                                }
 
                                public bool MoveNext ()
                                {
-                                       return host.MoveNext ();
+                                       if (!host.MoveNext ())
+                                               return false;
+                                       current = ((Node) host.Current).key;
+                                       return true;
                                }
 
                                public void Dispose ()
@@ -613,7 +628,10 @@ namespace System.Collections.Generic
                                }
 
                                object IEnumerator.Current {
-                                       get { return Current; }
+                                       get {
+                                               host.check_current ();
+                                               return current;
+                                       }
                                }
 
                                void IEnumerator.Reset ()
@@ -627,18 +645,24 @@ namespace System.Collections.Generic
                {
                        RBTree.NodeEnumerator host;
 
+                       KeyValuePair<TKey, TValue> current;
+
                        internal Enumerator (SortedDictionary<TKey,TValue> dic)
+                               : this ()
                        {
                                host = dic.tree.GetEnumerator ();
                        }
 
                        public KeyValuePair<TKey,TValue> Current {
-                               get { return ((Node) host.Current).AsKV (); }
+                               get { return current; }
                        }
 
                        public bool MoveNext ()
                        {
-                               return host.MoveNext ();
+                               if (!host.MoveNext ())
+                                       return false;
+                               current = ((Node) host.Current).AsKV ();
+                               return true;
                        }
 
                        public void Dispose ()
@@ -646,20 +670,27 @@ namespace System.Collections.Generic
                                host.Dispose ();
                        }
 
+                       Node CurrentNode {
+                               get {
+                                       host.check_current ();
+                                       return (Node) host.Current;
+                               }
+                       }
+
                        DictionaryEntry IDictionaryEnumerator.Entry {
-                               get { return ((Node) host.Current).AsDE (); }
+                               get { return CurrentNode.AsDE (); }
                        }
 
                        object IDictionaryEnumerator.Key {
-                               get { return ((Node) host.Current).key; }
+                               get { return CurrentNode.key; }
                        }
 
                        object IDictionaryEnumerator.Value {
-                               get { return ((Node) host.Current).value; }
+                               get { return CurrentNode.value; }
                        }
 
                        object IEnumerator.Current {
-                               get { return ((Node) host.Current).AsDE (); }
+                               get { return CurrentNode.AsDE (); }
                        }
 
                        void IEnumerator.Reset ()

diff --git a/mcs/class/System/Test/System.Collections.Generic/ChangeLog b/mcs/class/System/Test/System.Collections.Generic/ChangeLog
index 9901e47c71bf970070adc53cef7c509cd421ece6..35dea1ecd553fc3c1c5a21ef072f48e119c430a1 100644 (file)
--- a/mcs/class/System/Test/System.Collections.Generic/ChangeLog
+++ b/mcs/class/System/Test/System.Collections.Generic/ChangeLog
@@ -1,3 +1,8 @@
+2009-07-31  Raja R Harinath  <harinath@hurrynot.org>
+
+       * SortedDictionaryTest.cs (Enumerator_Current): New test.
+       (KeyEnumerator_Current, ValueEnumerator_Current): Likewise.
+
 2009-07-14 Gonzalo Paniagua Javier <gonzalo@novell.com>
 
        * SortedListTest.cs: new tests for bug #521750 provided by

diff --git a/mcs/class/System/Test/System.Collections.Generic/SortedDictionaryTest.cs b/mcs/class/System/Test/System.Collections.Generic/SortedDictionaryTest.cs
index af38b8dc7f876e4af719e55c6acc26fe509f100a..f293f329a20bc821ba89482eef8ff54eae77c318 100644 (file)
--- a/mcs/class/System/Test/System.Collections.Generic/SortedDictionaryTest.cs
+++ b/mcs/class/System/Test/System.Collections.Generic/SortedDictionaryTest.cs
@@ -463,6 +463,108 @@ namespace MonoTests.System.Collections.Generic
                        d.Add (4, "D");
                        e.MoveNext ();
                }
+
+
+               delegate void D ();
+               bool Throws (D d)
+               {
+                       try {
+                               d ();
+                               return false;
+                       } catch {
+                               return true;
+                       }
+               }
+
+               [Test]
+               // based on #491858, #517415
+               public void Enumerator_Current ()
+               {
+                       var e1 = new SortedDictionary<int,int>.Enumerator ();
+                       Assert.IsFalse (Throws (delegate { var x = e1.Current; }));
+
+                       var d = new SortedDictionary<int,int> ();
+                       var e2 = d.GetEnumerator ();
+                       Assert.IsFalse (Throws (delegate { var x = e2.Current; }));
+                       e2.MoveNext ();
+                       Assert.IsFalse (Throws (delegate { var x = e2.Current; }));
+                       e2.Dispose ();
+                       Assert.IsFalse (Throws (delegate { var x = e2.Current; }));
+
+                       var e3 = ((IEnumerable<KeyValuePair<int,int>>) d).GetEnumerator ();
+                       Assert.IsFalse (Throws (delegate { var x = e3.Current; }));
+                       e3.MoveNext ();
+                       Assert.IsFalse (Throws (delegate { var x = e3.Current; }));
+                       e3.Dispose ();
+                       Assert.IsFalse (Throws (delegate { var x = e3.Current; }));
+
+                       var e4 = ((IEnumerable) d).GetEnumerator ();
+                       Assert.IsTrue (Throws (delegate { var x = e4.Current; }));
+                       e4.MoveNext ();
+                       Assert.IsTrue (Throws (delegate { var x = e4.Current; }));
+                       ((IDisposable) e4).Dispose ();
+                       Assert.IsTrue (Throws (delegate { var x = e4.Current; }));
+               }
+
+               [Test]
+               // based on #491858, #517415
+               public void KeyEnumerator_Current ()
+               {
+                       var e1 = new SortedDictionary<int,int>.KeyCollection.Enumerator ();
+                       Assert.IsFalse (Throws (delegate { var x = e1.Current; }));
+
+                       var d = new SortedDictionary<int,int> ().Keys;
+                       var e2 = d.GetEnumerator ();
+                       Assert.IsFalse (Throws (delegate { var x = e2.Current; }));
+                       e2.MoveNext ();
+                       Assert.IsFalse (Throws (delegate { var x = e2.Current; }));
+                       e2.Dispose ();
+                       Assert.IsFalse (Throws (delegate { var x = e2.Current; }));
+
+                       var e3 = ((IEnumerable<int>) d).GetEnumerator ();
+                       Assert.IsFalse (Throws (delegate { var x = e3.Current; }));
+                       e3.MoveNext ();
+                       Assert.IsFalse (Throws (delegate { var x = e3.Current; }));
+                       e3.Dispose ();
+                       Assert.IsFalse (Throws (delegate { var x = e3.Current; }));
+
+                       var e4 = ((IEnumerable) d).GetEnumerator ();
+                       Assert.IsTrue (Throws (delegate { var x = e4.Current; }));
+                       e4.MoveNext ();
+                       Assert.IsTrue (Throws (delegate { var x = e4.Current; }));
+                       ((IDisposable) e4).Dispose ();
+                       Assert.IsTrue (Throws (delegate { var x = e4.Current; }));
+               }
+
+               [Test]
+               // based on #491858, #517415
+               public void ValueEnumerator_Current ()
+               {
+                       var e1 = new SortedDictionary<int,int>.ValueCollection.Enumerator ();
+                       Assert.IsFalse (Throws (delegate { var x = e1.Current; }));
+
+                       var d = new SortedDictionary<int,int> ().Values;
+                       var e2 = d.GetEnumerator ();
+                       Assert.IsFalse (Throws (delegate { var x = e2.Current; }));
+                       e2.MoveNext ();
+                       Assert.IsFalse (Throws (delegate { var x = e2.Current; }));
+                       e2.Dispose ();
+                       Assert.IsFalse (Throws (delegate { var x = e2.Current; }));
+
+                       var e3 = ((IEnumerable<int>) d).GetEnumerator ();
+                       Assert.IsFalse (Throws (delegate { var x = e3.Current; }));
+                       e3.MoveNext ();
+                       Assert.IsFalse (Throws (delegate { var x = e3.Current; }));
+                       e3.Dispose ();
+                       Assert.IsFalse (Throws (delegate { var x = e3.Current; }));
+
+                       var e4 = ((IEnumerable) d).GetEnumerator ();
+                       Assert.IsTrue (Throws (delegate { var x = e4.Current; }));
+                       e4.MoveNext ();
+                       Assert.IsTrue (Throws (delegate { var x = e4.Current; }));
+                       ((IDisposable) e4).Dispose ();
+                       Assert.IsTrue (Throws (delegate { var x = e4.Current; }));
+               }
        }
 
        class ReverseComparer<T> : IComparer<T>