Fix failing assertions in GetInternalIteratorSequenceStorage

Author: steve-s

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/SequenceStorageNodes.java

@@ -4059,21 +4059,23 @@ public abstract static class CreateStorageFromIteratorNodeCached extends CreateS
 
             @CompilationFinal private ListStorageType expectedElementType = Uninitialized;
 
+            private static final int MAX_PREALLOCATE_SIZE = 32;
             @CompilationFinal int startSizeProfiled = START_SIZE;
 
             public boolean isBuiltinIterator(Object iterator) {
                 return iterator instanceof PBuiltinIterator && getClass.execute((PBuiltinIterator) iterator) == PythonBuiltinClassType.PIterator;
             }
 
             public static SequenceStorage getSequenceStorage(GetInternalIteratorSequenceStorage node, PBuiltinIterator iterator) {
-                return node.execute(iterator);
+                return iterator.index != 0 || iterator.isExhausted() ? null : node.execute(iterator);
             }
 
-            @Specialization(guards = {"isBuiltinIterator(it)", "it.index == 0", "storage != null"})
+            @Specialization(guards = {"isBuiltinIterator(it)", "storage != null"})
             public SequenceStorage createBuiltinFastPath(PBuiltinIterator it, int len,
                             @Cached GetInternalIteratorSequenceStorage getIterSeqStorageNode,
                             @Bind("getSequenceStorage(getIterSeqStorageNode, it)") SequenceStorage storage,
                             @Cached CopyNode copyNode) {
+                it.setExhausted();
                 return copyNode.execute(storage);
             }
 
@@ -4085,7 +4087,7 @@ public SequenceStorage createBuiltinUnknownLen(VirtualFrame frame, PBuiltinItera
                             @Shared("arrayGrowProfile") @Cached("createCountingProfile()") ConditionProfile arrayGrowProfile,
                             @Cached NextNode nextNode) {
                 int expectedLen = lengthHint.execute(iterator);
-                if (expectedLen == -1) {
+                if (expectedLen < 0) {
                     expectedLen = startSizeProfiled;
                 }
                 SequenceStorage s = createStorageFromBuiltin(frame, iterator, expectedLen, expectedElementType, nextNode, errorProfile, arrayGrowProfile, loopProfile);
@@ -4123,7 +4125,7 @@ public SequenceStorage createGenericKnownLen(VirtualFrame frame, Object iterator
             private SequenceStorage profileResult(SequenceStorage storage, boolean profileLength) {
                 if (CompilerDirectives.inInterpreter() && profileLength) {
                     int actualLen = storage.length();
-                    if (startSizeProfiled < actualLen && actualLen <= 32) {
+                    if (startSizeProfiled < actualLen && actualLen <= MAX_PREALLOCATE_SIZE) {
                         startSizeProfiled = actualLen;
                     }
                 }
@@ -4152,7 +4154,7 @@ public SequenceStorage execute(VirtualFrame frame, Object iterator, int len) {
             private SequenceStorage executeImpl(Object iterator, int len) {
                 if (iterator instanceof PBuiltinIterator) {
                     PBuiltinIterator pbi = (PBuiltinIterator) iterator;
-                    if (GetClassNode.getUncached().execute(pbi) == PythonBuiltinClassType.PIterator && pbi.index == 0) {
+                    if (GetClassNode.getUncached().execute(pbi) == PythonBuiltinClassType.PIterator && pbi.index == 0 && !pbi.isExhausted()) {
                         SequenceStorage s = GetInternalIteratorSequenceStorage.getUncached().execute(pbi);
                         if (s != null) {
                             return s.copy();

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/iterator/IteratorNodes.java

@@ -80,6 +80,7 @@
 import com.oracle.graal.python.runtime.sequence.storage.SequenceStorage;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
+import com.oracle.truffle.api.dsl.Bind;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Fallback;
 import com.oracle.truffle.api.dsl.GenerateNodeFactory;
@@ -188,6 +189,7 @@ int length(VirtualFrame frame, Object iterable,
     }
 
     @ImportStatic(PGuards.class)
+    @GenerateUncached
     public abstract static class GetInternalIteratorSequenceStorage extends Node {
         public static GetInternalIteratorSequenceStorage getUncached() {
             return GetInternalIteratorSequenceStorageNodeGen.getUncached();
@@ -200,7 +202,7 @@ public static GetInternalIteratorSequenceStorage getUncached() {
          */
         public final SequenceStorage execute(PBuiltinIterator iterator) {
             assert GetClassNode.getUncached().execute(iterator) == PIterator;
-            assert iterator.index == 0;
+            assert iterator.index == 0 && !iterator.isExhausted();
             return executeInternal(iterator);
         }
 
@@ -244,43 +246,54 @@ static SequenceStorage doOthers(PBuiltinIterator it) {
 
     @ImportStatic(PGuards.class)
     public abstract static class BuiltinIteratorLengthHint extends Node {
-        @Child GetInternalIteratorSequenceStorage getSeqStorage = GetInternalIteratorSequenceStorageNodeGen.create();
-        private final ConditionProfile noStorageProfile = ConditionProfile.createBinaryProfile();
-
         /**
          * The argument must be a builtin iterator. Returns {@code -1} if the length hint is not
-         * available.
+         * available and rewrites itself to generic fallback that always returns {@code -1}.
          */
         public final int execute(PBuiltinIterator iterator) {
             assert GetClassNode.getUncached().execute(iterator) == PIterator;
-            SequenceStorage result = getSeqStorage.execute(iterator);
-            if (noStorageProfile.profile(result != null)) {
-                return result.length();
-            }
             return executeInternal(iterator);
         }
 
         protected abstract int executeInternal(PBuiltinIterator iterator);
 
+        protected static SequenceStorage getStorage(GetInternalIteratorSequenceStorage getSeqStorage, PBuiltinIterator it) {
+            return it.index != 0 || it.isExhausted() ? null : getSeqStorage.execute(it);
+        }
+
+        @Specialization(guards = "storage != null")
+        static int doSeqStorage(@SuppressWarnings("unused") PBuiltinIterator it,
+                        @SuppressWarnings("unused") @Cached GetInternalIteratorSequenceStorage getSeqStorage,
+                        @Bind("getStorage(getSeqStorage, it)") SequenceStorage storage) {
+            return ensurePositive(storage.length());
+        }
+
         @Specialization
         static int doString(PStringIterator it) {
-            return it.value.length();
+            return ensurePositive(it.value.length());
         }
 
         @Specialization
         static int doSequenceArr(PArrayIterator it) {
-            return it.array.getLength();
+            return ensurePositive(it.array.getLength());
         }
 
         @Specialization
         static int doSequenceIntRange(PIntRangeIterator it) {
-            return it.getLength();
+            return ensurePositive(it.getLength());
         }
 
-        @Fallback
-        static int doOthers(PBuiltinIterator it) {
+        @Specialization(replaces = {"doSeqStorage", "doString", "doSequenceArr", "doSequenceIntRange"})
+        static int doGeneric(@SuppressWarnings("unused") PBuiltinIterator it) {
             return -1;
         }
+
+        static int ensurePositive(int len) {
+            if (len < 0) {
+                throw CompilerDirectives.shouldNotReachHere();
+            }
+            return len;
+        }
     }
 
     @GenerateUncached