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

5 files changed, 171 insertions, 21 deletions

diff --git a/mcs/class/System/System.Collections.Generic/ChangeLog b/mcs/class/System/System.Collections.Generic/ChangeLog
index 0bfc5fbdfd1..ba7608dda77 100644
--- 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 bf4b64f616e..6325f32ff2e 100644
--- 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 3f75cf24d9e..95965c9d563 100644
--- 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 9901e47c71b..35dea1ecd55 100644
--- 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 af38b8dc7f8..f293f329a20 100644
--- 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>