Merge pull request #1293 from kmod/str_iter

str does not have `__iter__`

src/codegen/compvars.cpp

@@ -438,8 +438,13 @@ public:
         // var has no __iter__()
         // TODO: we could create a patchpoint if this turns out to be hot
         emitter.setCurrentBasicBlock(bb_no_iter);
-        llvm::Value* value_no_iter = emitter.createCall(info.unw_info, g.funcs.getiterHelper, var->getValue());
+
+        ICSetupInfo* pp2 = createGenericIC(info.getTypeRecorder(), true, 128);
+        llvm::Instruction* value_no_iter
+            = emitter.createIC(pp2, (void*)getiterHelper, { var->getValue() }, info.unw_info);
+        value_no_iter = createAfter<llvm::IntToPtrInst>(value_no_iter, value_no_iter, g.llvm_value_type_ptr, "");
         emitter.setType(value_no_iter, RefType::OWNED);
+
         llvm::BasicBlock* value_no_iter_bb = emitter.currentBasicBlock();
         auto no_iter_terminator = emitter.getBuilder()->CreateBr(bb_join);
 
@@ -1987,7 +1992,7 @@ public:
         CompilerVariable* called_constant = tryCallattrConstant(emitter, info, var, attr, flags.cls_only, flags.argspec,
                                                                 args, keyword_names, no_attribute_ptr, exception_style);
 
-        if (no_attribute)
+        if (flags.null_on_nonexistent && no_attribute)
             return new ConcreteCompilerVariable(UNKNOWN, getNullPtr(g.llvm_value_type_ptr));
 
         if (called_constant)

src/core/types.h

@@ -682,6 +682,7 @@ extern HiddenClass* root_hcls;
 struct SetattrRewriteArgs;
 struct GetattrRewriteArgs;
 struct DelattrRewriteArgs;
+struct UnaryopRewriteArgs;
 
 // Helper function around PyString_InternFromString:
 BoxedString* internStringImmortal(llvm::StringRef s) noexcept;

src/runtime/iterobject.cpp

@@ -40,39 +40,47 @@ Box* seqiterIter(Box* s) {
     return incref(s);
 }
 
-static Box* seqiterHasnext_capi(Box* s) noexcept {
+static llvm_compat_bool seqiterHasnextUnboxed(Box* s) {
     RELEASE_ASSERT(s->cls == seqiter_cls || s->cls == seqreviter_cls, "");
     BoxedSeqIter* self = static_cast<BoxedSeqIter*>(s);
 
-    if (!self->b) {
-        Py_RETURN_FALSE;
+    if (!self->b)
+        return false;
+
+    if (self->len != -1) {
+        if (self->idx >= self->len)
+            return false;
+        assert(!self->next);
+        if (self->b->cls == str_cls)
+            self->next = incref(characters[static_cast<BoxedString*>(self->b)->s()[self->idx] & UCHAR_MAX]);
+        else if (self->b->cls == unicode_cls)
+            self->next = PyUnicode_FromUnicode(&reinterpret_cast<PyUnicodeObject*>(self->b)->str[self->idx], 1);
+        else
+            self->next = PySequence_GetItem(self->b, self->idx);
+        assert(self->next);
+        self->idx++;
+        return true;
     }
 
+    // TODO: inline PySequence_GetItem
     Box* next = PySequence_GetItem(self->b, self->idx);
     if (!next) {
         if (PyErr_ExceptionMatches(IndexError) || PyErr_ExceptionMatches(StopIteration)) {
             PyErr_Clear();
             Py_CLEAR(self->b);
-            Py_RETURN_FALSE;
+            return false;
         }
-        return NULL;
+        throwCAPIException();
     }
 
     self->idx++;
     RELEASE_ASSERT(!self->next, "");
     self->next = next;
-    Py_RETURN_TRUE;
+    return true;
 }
 
 Box* seqiterHasnext(Box* s) {
-    Box* rtn = seqiterHasnext_capi(s);
-    if (!rtn)
-        throwCAPIException();
-    return rtn;
-}
-
-llvm_compat_bool seqiterHasnextUnboxed(Box* s) {
-    return unboxBool(autoDecref(seqiterHasnext(s)));
+    return boxBool(seqiterHasnextUnboxed(s));
 }
 
 Box* seqreviterHasnext_capi(Box* s) noexcept {
@@ -109,15 +117,20 @@ Box* seqiter_next(Box* s) noexcept {
     BoxedSeqIter* self = static_cast<BoxedSeqIter*>(s);
 
     if (!self->next) {
-        Box* hasnext = NULL;
-        if (s->cls == seqiter_cls)
-            hasnext = seqiterHasnext_capi(s);
-        else if (s->cls == seqreviter_cls)
-            hasnext = seqreviterHasnext_capi(s);
-        else
-            RELEASE_ASSERT(0, "");
-        AUTO_XDECREF(hasnext);
-        if (hasnext != Py_True)
+        bool hasnext;
+        try {
+            if (s->cls == seqiter_cls)
+                hasnext = seqiterHasnextUnboxed(s);
+            else if (s->cls == seqreviter_cls)
+                hasnext = unboxBool(autoDecref(seqreviterHasnext(s)));
+            else
+                RELEASE_ASSERT(0, "");
+        } catch (ExcInfo e) {
+            setCAPIException(e);
+            return NULL;
+        }
+
+        if (!hasnext)
             return NULL;
     }
 
@@ -198,8 +211,9 @@ void setupIter() {
                                      (traverseproc)BoxedSeqIter::traverse, NOCLEAR);
 
     seqiter_cls->giveAttr("next", new BoxedFunction(FunctionMetadata::create((void*)seqiterNext, UNKNOWN, 1)));
-    seqiter_cls->giveAttr("__hasnext__",
-                          new BoxedFunction(FunctionMetadata::create((void*)seqiterHasnext, BOXED_BOOL, 1)));
+    FunctionMetadata* hasnext = FunctionMetadata::create((void*)seqiterHasnextUnboxed, BOOL, 1);
+    hasnext->addVersion((void*)seqiterHasnext, BOXED_BOOL);
+    seqiter_cls->giveAttr("__hasnext__", new BoxedFunction(hasnext));
     seqiter_cls->giveAttr("__iter__", new BoxedFunction(FunctionMetadata::create((void*)seqiterIter, UNKNOWN, 1)));
 
     seqiter_cls->freeze();

src/runtime/iterobject.h

@@ -33,9 +33,24 @@ public:
     int64_t idx;
     Box* next;
 
-    BoxedSeqIter(Box* b, int64_t start) : b(b), idx(start), next(NULL) { Py_INCREF(b); }
+    // Pyston change:
+    // For types that allow it, this class will do the more efficient length-based
+    // iteration, storing the length here.  Otherwise len is -1.
+    int64_t len;
+
+    BoxedSeqIter(Box* b, int64_t start) : b(b), idx(start), next(NULL) {
+        Py_INCREF(b);
+
+        if (b->cls == str_cls) {
+            len = static_cast<BoxedString*>(b)->size();
+        } else if (b->cls == unicode_cls) {
+            len = reinterpret_cast<PyUnicodeObject*>(b)->length;
+        } else {
+            len = -1;
+        }
+    }
 
-    DEFAULT_CLASS(seqiter_cls);
+    DEFAULT_CLASS_SIMPLE(seqiter_cls, true);
 
     static void dealloc(BoxedSeqIter* o) noexcept {
         PyObject_GC_UnTrack(o);

src/runtime/objmodel.cpp

@@ -3466,7 +3466,7 @@ extern "C" BoxedInt* hash(Box* obj) {
 }
 
 template <ExceptionStyle S, Rewritable rewritable>
-BoxedInt* lenInternal(Box* obj, LenRewriteArgs* rewrite_args) noexcept(S == CAPI) {
+BoxedInt* lenInternal(Box* obj, UnaryopRewriteArgs* rewrite_args) noexcept(S == CAPI) {
     if (rewritable == NOT_REWRITABLE) {
         assert(!rewrite_args);
         rewrite_args = NULL;
@@ -3594,10 +3594,10 @@ BoxedInt* lenInternal(Box* obj, LenRewriteArgs* rewrite_args) noexcept(S == CAPI
 }
 
 // force template instantiation:
-template BoxedInt* lenInternal<CAPI, REWRITABLE>(Box*, LenRewriteArgs*);
-template BoxedInt* lenInternal<CXX, REWRITABLE>(Box*, LenRewriteArgs*);
-template BoxedInt* lenInternal<CAPI, NOT_REWRITABLE>(Box*, LenRewriteArgs*);
-template BoxedInt* lenInternal<CXX, NOT_REWRITABLE>(Box*, LenRewriteArgs*);
+template BoxedInt* lenInternal<CAPI, REWRITABLE>(Box*, UnaryopRewriteArgs*);
+template BoxedInt* lenInternal<CXX, REWRITABLE>(Box*, UnaryopRewriteArgs*);
+template BoxedInt* lenInternal<CAPI, NOT_REWRITABLE>(Box*, UnaryopRewriteArgs*);
+template BoxedInt* lenInternal<CXX, NOT_REWRITABLE>(Box*, UnaryopRewriteArgs*);
 
 Box* lenCallInternal(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Box* arg1, Box* arg2,
                      Box* arg3, Box** args, const std::vector<BoxedString*>* keyword_names) {
@@ -3605,7 +3605,7 @@ Box* lenCallInternal(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args, Arg
         return callFunc<CXX>(func, rewrite_args, argspec, arg1, arg2, arg3, args, keyword_names);
 
     if (rewrite_args) {
-        LenRewriteArgs lrewrite_args(rewrite_args->rewriter, rewrite_args->arg1, rewrite_args->destination);
+        UnaryopRewriteArgs lrewrite_args(rewrite_args->rewriter, rewrite_args->arg1, rewrite_args->destination);
         Box* rtn = lenInternal<CXX, REWRITABLE>(arg1, &lrewrite_args);
         if (!lrewrite_args.out_success) {
             rewrite_args = 0;
@@ -3640,7 +3640,7 @@ extern "C" i64 unboxedLen(Box* obj) {
     RewriterVar* r_boxed = NULL;
     if (rewriter.get()) {
         // rewriter->trap();
-        LenRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getReturnDestination());
+        UnaryopRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getReturnDestination());
         lobj = lenInternal<CXX, REWRITABLE>(obj, &rewrite_args);
 
         if (!rewrite_args.out_success) {
@@ -6804,14 +6804,50 @@ extern "C" Box* getPystonIter(Box* o) {
     return r;
 }
 
-extern "C" Box* getiterHelper(Box* o) {
-    if (PySequence_Check(o))
+extern "C" Box* getiterHelperInternal(Box* o, UnaryopRewriteArgs* rewrite_args) {
+    if (rewrite_args)
+        assert(o->cls->is_constant);
+
+    if (PySequence_Check(o)) {
+        class Helper {
+        public:
+            static Box* call(Box* o) { return new BoxedSeqIter(o, 0); }
+        };
+
+        if (rewrite_args) {
+            rewrite_args->out_rtn
+                = rewrite_args->rewriter->call(false, (void*)&Helper::call, rewrite_args->obj)->setType(RefType::OWNED);
+            rewrite_args->out_success = true;
+        }
         return new BoxedSeqIter(o, 0);
+    }
+
     raiseExcHelper(TypeError, "'%s' object is not iterable", getTypeName(o));
 }
 
+extern "C" Box* getiterHelper(Box* o) {
+    std::unique_ptr<Rewriter> rewriter(nullptr);
+
+    if (o->cls->is_constant)
+        rewriter.reset(
+            Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 1, "getPystonIter"));
+
+    if (rewriter.get()) {
+        UnaryopRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0)->setType(RefType::BORROWED),
+                                        rewriter->getReturnDestination());
+
+        Box* r = getiterHelperInternal(o, &rewrite_args);
+        if (rewrite_args.out_success) {
+            RewriterVar* r_rtn = rewrite_args.out_rtn;
+            rewriter->commitReturning(r_rtn);
+        }
+        return r;
+    } else {
+        return getiterHelperInternal(o, NULL);
+    }
+}
+
 Box* getiter(Box* o) {
-    // TODO add rewriting to this?  probably want to try to avoid this path though
     BoxedClass* type = o->cls;
     Box* r = NULL;
     if (PyType_HasFeature(type, Py_TPFLAGS_HAVE_ITER) && type->tp_iter != slot_tp_iter && type->tp_iter) {
@@ -6828,7 +6864,7 @@ Box* getiter(Box* o) {
         }
         return r;
     }
-    return getiterHelper(o);
+    return getiterHelperInternal(o, NULL);
 }
 
 void assertValidSlotIdentifier(Box* s) {

src/runtime/objmodel.h

@@ -110,7 +110,7 @@ extern "C" BoxedClosure* createClosure(BoxedClosure* parent_closure, size_t size
 
 Box* getiter(Box* o);
 extern "C" Box* getPystonIter(Box* o);
-extern "C" Box* getiterHelper(Box* o);
+extern "C" Box* getiterHelper(Box* o) __attribute__((noinline));
 extern "C" Box* createBoxedIterWrapperIfNeeded(Box* o) __attribute__((noinline));
 
 struct SetattrRewriteArgs;
@@ -133,9 +133,8 @@ template <ExceptionStyle S> inline Box* getitemInternal(Box* target, Box* slice)
     return getitemInternal<S, NOT_REWRITABLE>(target, slice, NULL);
 }
 
-struct LenRewriteArgs;
 template <ExceptionStyle S, Rewritable rewritable>
-BoxedInt* lenInternal(Box* obj, LenRewriteArgs* rewrite_args) noexcept(S == CAPI);
+BoxedInt* lenInternal(Box* obj, UnaryopRewriteArgs* rewrite_args) noexcept(S == CAPI);
 Box* lenCallInternal(BoxedFunctionBase* f, CallRewriteArgs* rewrite_args, ArgPassSpec argspec, Box* arg1, Box* arg2,
                      Box* arg3, Box** args, const std::vector<BoxedString*>* keyword_names);
 

src/runtime/rewrite_args.h

@@ -194,7 +194,7 @@ struct DelattrRewriteArgs {
     DelattrRewriteArgs(Rewriter* rewriter, RewriterVar* obj) : rewriter(rewriter), obj(obj), out_success(false) {}
 };
 
-struct LenRewriteArgs {
+struct UnaryopRewriteArgs {
     Rewriter* rewriter;
     RewriterVar* obj;
     Location destination;
@@ -202,7 +202,7 @@ struct LenRewriteArgs {
     bool out_success;
     RewriterVar* out_rtn;
 
-    LenRewriteArgs(Rewriter* rewriter, RewriterVar* obj, Location destination)
+    UnaryopRewriteArgs(Rewriter* rewriter, RewriterVar* obj, Location destination)
         : rewriter(rewriter), obj(obj), destination(destination), out_success(false), out_rtn(NULL) {}
 };
 

src/runtime/str.cpp

@@ -2406,75 +2406,6 @@ extern "C" Box* strGetslice(BoxedString* self, Box* boxedStart, Box* boxedStop)
     return _strSlice(self, start, stop, 1, stop - start);
 }
 
-
-// TODO it looks like strings don't have their own iterators, but instead
-// rely on the sequence iteration protocol.
-// Should probably implement that, and maybe once that's implemented get
-// rid of the striterator class?
-BoxedClass* str_iterator_cls = NULL;
-
-class BoxedStringIterator : public Box {
-public:
-    BoxedString* s;
-    std::string::const_iterator it, end;
-
-    BoxedStringIterator(BoxedString* s) : s(s), it(s->s().begin()), end(s->s().end()) { Py_INCREF(s); }
-
-    DEFAULT_CLASS(str_iterator_cls);
-
-    static llvm_compat_bool hasnextUnboxed(BoxedStringIterator* self) {
-        assert(self->cls == str_iterator_cls);
-        return self->it != self->end;
-    }
-
-    static Box* hasnext(BoxedStringIterator* self) {
-        assert(self->cls == str_iterator_cls);
-        return boxBool(self->it != self->end);
-    }
-
-    static Box* iter(BoxedStringIterator* self) {
-        assert(self->cls == str_iterator_cls);
-        return incref(self);
-    }
-
-    static Box* next(BoxedStringIterator* self) {
-        assert(self->cls == str_iterator_cls);
-        if (!hasnextUnboxed(self))
-            raiseExcHelper(StopIteration, (const char*)nullptr);
-
-        char c = *self->it;
-        ++self->it;
-        return incref(characters[c & UCHAR_MAX]);
-    }
-
-    static Box* next_capi(Box* _self) noexcept {
-        assert(_self->cls == str_iterator_cls);
-        auto self = (BoxedStringIterator*)_self;
-        if (!hasnextUnboxed(self))
-            return NULL;
-
-        char c = *self->it;
-        ++self->it;
-        return incref(characters[c & UCHAR_MAX]);
-    }
-
-    static void dealloc(BoxedStringIterator* o) noexcept {
-        PyObject_GC_UnTrack(o);
-        Py_DECREF(o->s);
-        o->cls->tp_free(o);
-    }
-
-    static int traverse(BoxedStringIterator* self, visitproc visit, void* arg) noexcept {
-        Py_VISIT(self->s);
-        return 0;
-    }
-};
-
-Box* strIter(BoxedString* self) noexcept {
-    assert(PyString_Check(self));
-    return new BoxedStringIterator(self);
-}
-
 extern "C" PyObject* PyString_FromString(const char* s) noexcept {
     return boxString(s);
 }
@@ -2860,20 +2791,6 @@ void setupStr() {
 
     assert(str_cls->tp_basicsize == PyStringObject_SIZE);
 
-    str_iterator_cls = BoxedClass::create(type_cls, object_cls, 0, 0, sizeof(BoxedStringIterator), false, "striterator",
-                                          false, (destructor)BoxedStringIterator::dealloc, NULL, true,
-                                          (traverseproc)BoxedStringIterator::traverse, NOCLEAR);
-    str_iterator_cls->giveAttr(
-        "__hasnext__", new BoxedFunction(FunctionMetadata::create((void*)BoxedStringIterator::hasnext, BOXED_BOOL, 1)));
-    str_iterator_cls->giveAttr(
-        "__iter__", new BoxedFunction(FunctionMetadata::create((void*)BoxedStringIterator::iter, UNKNOWN, 1)));
-    str_iterator_cls->giveAttr("next",
-                               new BoxedFunction(FunctionMetadata::create((void*)BoxedStringIterator::next, STR, 1)));
-    str_iterator_cls->freeze();
-    str_iterator_cls->tpp_hasnext = (BoxedClass::pyston_inquiry)BoxedStringIterator::hasnextUnboxed;
-    str_iterator_cls->tp_iternext = BoxedStringIterator::next_capi;
-    str_iterator_cls->tp_iter = PyObject_SelfIter;
-
     str_cls->tp_as_buffer = &string_as_buffer;
 
     str_cls->giveAttr("__getnewargs__", new BoxedFunction(FunctionMetadata::create((void*)string_getnewargs, UNKNOWN, 1,
@@ -2953,9 +2870,6 @@ void setupStr() {
 
     str_cls->giveAttr("__getslice__", new BoxedFunction(FunctionMetadata::create((void*)strGetslice, STR, 3)));
 
-    str_cls->giveAttr("__iter__",
-                      new BoxedFunction(FunctionMetadata::create((void*)strIter, typeFromClass(str_iterator_cls), 1)));
-
     add_methods(str_cls, string_methods);
 
     auto str_new = FunctionMetadata::create((void*)strNew<CXX>, UNKNOWN, 2, false, false,
@@ -2978,7 +2892,6 @@ void setupStr() {
     str_cls->tp_repr = str_repr;
     str_cls->tp_str = str_str;
     str_cls->tp_print = string_print;
-    str_cls->tp_iter = (decltype(str_cls->tp_iter))strIter;
     str_cls->tp_hash = (hashfunc)str_hash;
     str_cls->tp_as_sequence->sq_concat = (binaryfunc)strAdd<CAPI>;
     str_cls->tp_as_sequence->sq_contains = (objobjproc)string_contains;

test/extra/formencode.patch

+--- src/formencode/formencode/validators.py.orig	2016-07-14 17:02:54.061523151 +0000
++++ src/formencode/formencode/validators.py	2016-07-14 17:03:55.577523151 +0000
+@@ -1394,14 +1394,10 @@
+         'http://google.com'
+         >>> u.to_python('google.com')
+         Traceback (most recent call last):
+             ...
+         Invalid: You must start your URL with http://, https://, etc
+-        >>> u.to_python('http://www.formencode.org/does/not/exist/page.html')
+-        Traceback (most recent call last):
+-            ...
+-        Invalid: The server responded that the page could not be found
+         >>> u.to_python('http://this.domain.does.not.exist.example.org/test.html')
+         ... # doctest: +ELLIPSIS
+         Traceback (most recent call last):
+             ...
+         Invalid: An error occured when trying to connect to the server: ...

test/extra/formencode_test.py

@@ -13,5 +13,7 @@ packages = ["nose==1.3.7", "pycountry==1.6", "pyDNS==2.3.6"]
 packages += ["-e", "git+https://github.com/formencode/formencode.git@1.2.5#egg=formencode"]
 create_virtenv(ENV_NAME, packages, force_create = True)
 
+subprocess.check_call(["patch", "-p1"], stdin=open(os.path.join(os.path.dirname(__file__), "formencode.patch")), cwd=SRC_DIR)
+
 expected = [{'ran': 201}]
 run_test([NOSETESTS_EXE], cwd=FORMENCODE_DIR, expected=expected)

test/extra/paste_test.py

@@ -36,7 +36,7 @@ print ">> "
 # - no sys.settrace 
 # - no shiftjis encoding
 # - slightly different error messages
-expected = [{"failed" : 22, "passed" : 112}]
+expected = [{"failed" : 21, "passed" : 113}]
 run_test([PYTEST_EXE], cwd=PASTE_TEST_DIR, expected=expected)
 
 

test/tests/str_functions.py

@@ -221,3 +221,6 @@ class D(str):
     pass
 
 print(D('abc').__rmod__('%s'))
+
+# Real code needs str to not have an iter!
+print hasattr("", "__iter__")

test/tests/str_iterator.py

-# expected: fail
 print type(iter("hello world"))