Ensure only implementers of IEnumerable or IEnumerator are considered Iterable (#1241)

danabr · web-flow · commit b5ba81572e6d · 2020-09-29T13:46:31.000-07:00
Fixes #1234. Previously `tp_iter` slot was set for all reflected .NET types. This restricts it to types, that implement `IEnumerable` or `IEnumerator`
diff --git a/AUTHORS.md b/AUTHORS.md
@@ -28,6 +28,7 @@
 -   Christoph Gohlke ([@cgohlke](https://github.com/cgohlke))
 -   Christopher Bremner ([@chrisjbremner](https://github.com/chrisjbremner))
 -   Christopher Pow ([@christopherpow](https://github.com/christopherpow))
+-   Daniel Abrahamsson ([@danabr](https://github.com/danabr))
 -   Daniel Fernandez ([@fdanny](https://github.com/fdanny))
 -   Daniel Santana ([@dgsantana](https://github.com/dgsantana))
 -   Dave Hirschfeld ([@dhirschfeld](https://github.com/dhirschfeld))
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,7 @@ details about the cause of the failure
 -    Fix incorrect dereference of wrapper object in `tp_repr`, which may result in a program crash
 -    Fix incorrect dereference in params array handling
 -    Fix `object[]` parameters taking precedence when should not in overload resolution
+-    Fixed a bug where all .NET class instances were considered Iterable
 
 ## [2.5.0][] - 2020-06-14
 
diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs
@@ -164,6 +164,13 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType)
             // we want to do this after the slot stuff above in case the class itself implements a slot method
             InitializeSlots(type, impl.GetType());
 
+            if (!clrType.GetInterfaces().Any(ifc => ifc == typeof(IEnumerable) || ifc == typeof(IEnumerator)))
+            {
+                // The tp_iter slot should only be set for enumerable types.
+                Marshal.WriteIntPtr(type, TypeOffset.tp_iter, IntPtr.Zero);
+            }
+
+
             if (base_ != IntPtr.Zero)
             {
                 Marshal.WriteIntPtr(type, TypeOffset.tp_base, base_);
diff --git a/src/tests/test_class.py b/src/tests/test_class.py
@@ -172,6 +172,23 @@ def test_ienumerator_iteration():
     for item in chars:
         assert item in 'test string'
 
+def test_iterable():
+    """Test what objects are Iterable"""
+    from collections.abc import Iterable
+    from Python.Test import ClassTest
+
+    assert isinstance(System.String.Empty, Iterable)
+    assert isinstance(ClassTest.GetArrayList(), Iterable)
+    assert isinstance(ClassTest.GetEnumerator(), Iterable)
+    assert (not isinstance(ClassTest, Iterable))
+    assert (not isinstance(ClassTest(), Iterable))
+
+    class ShouldBeIterable(ClassTest):
+        def __iter__(self):
+            return iter([])
+
+    assert isinstance(ShouldBeIterable(), Iterable)
+
 
 def test_override_get_item():
     """Test managed subclass overriding __getitem__."""
