Changed UnicodeBuilders to operate on a c string instead of a list of Python strings

Author: cklein

hpy/debug/src/autogen_debug_ctx_init.h

@@ -7,8 +7,7 @@
 
 
 _HPy_HIDDEN HPyUnicodeBuilder ctx_UnicodeBuilder_New(HPyContext *ctx, HPy_ssize_t capacity);
-_HPy_HIDDEN int ctx_UnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder,
-                                       HPy h_item);
+_HPy_HIDDEN int ctx_UnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, const char *item);
 _HPy_HIDDEN HPy ctx_UnicodeBuilder_Build(HPyContext *ctx, HPyUnicodeBuilder builder);
 _HPy_HIDDEN void ctx_UnicodeBuilder_Cancel(HPyContext *ctx, HPyUnicodeBuilder builder);
 

hpy/debug/src/autogen_debug_wrappers.c

@@ -442,9 +442,9 @@ HPyUnicodeBuilder_New(HPyContext *ctx, HPy_ssize_t size)
 }
 
 HPyAPI_FUNC(int)
-HPyUnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, HPy h_item)
+HPyUnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, const char *item)
 {
-    return ctx_UnicodeBuilder_Add(ctx, builder, h_item);
+    return ctx_UnicodeBuilder_Add(ctx, builder, item);
 }
 
 HPyAPI_FUNC(HPy)

hpy/devel/include/common/runtime/ctx_unicodebuilder.h

@@ -1,4 +1,5 @@
 #include <stddef.h>
+#include <string.h>
 #include <Python.h>
 #include "hpy.h"
 
@@ -7,12 +8,12 @@
 #  include "handles.h"
 #endif
 
-static const Py_ssize_t HPYUNICODEBUILDER_INITIAL_CAPACITY = 5;
+static const Py_ssize_t HPYUNICODEBUILDER_INITIAL_CAPACITY = 1024;
 
 typedef struct {
     Py_ssize_t capacity;  // allocated handles
-    Py_ssize_t length;    // used handles
-    PyObject *list;
+    Py_ssize_t length;
+    char *buf;
 } _PyUnicodeBuilder_s;
 
 #ifdef HPY_UNIVERSAL_ABI
@@ -36,72 +37,65 @@ ctx_UnicodeBuilder_New(HPyContext *ctx, HPy_ssize_t capacity)
 {
     _PyUnicodeBuilder_s *bp;
     if (capacity == 0) {
+        // TODO: default value or raise a ValueError?
         capacity = HPYUNICODEBUILDER_INITIAL_CAPACITY;
     }
-    capacity++; // always reserve space for an extra handle, see the docs, analogue to HPyTracker
+    capacity++; // always reserve space for the trailing 0
 
     bp = malloc(sizeof(_PyUnicodeBuilder_s));
     if (bp == NULL) {
         HPyErr_NoMemory(ctx);
         return _pb2hb(0);
     }
 
-    bp->list = PyList_New(capacity);
-    if (bp->list == NULL) {
+    bp->buf = malloc(capacity);
+    if (bp == NULL) {
         free(bp);
         HPyErr_NoMemory(ctx);
         return _pb2hb(0);
     }
+
     bp->capacity = capacity;
     bp->length = 0;
     return _pb2hb(bp);
 }
 
 _HPy_HIDDEN int
-ctx_UnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, HPy h_item)
+ctx_UnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, const char *s)
 {
-    if(!HPyUnicode_Check(ctx, h_item)) {
-        HPyErr_SetString(ctx, ctx->h_TypeError, "Argument must be of type HPyUnicode");
-        return -1;
-    }
-
     _PyUnicodeBuilder_s *bp = _hb2pb(builder);
-    PyObject *item = _h2py(h_item);
-
-    // XXX: For the initial PoC we don't care about reallocation
-    if (bp->capacity <= bp->length) {
-        return -1;
+    // TODO: Should we trust the user to submit a 0 terminated string?
+    // The alternative would be to use strnlen and have a maximum allowed length for s
+    int len = strlen(s);
+    if(bp->length + len >= bp->capacity) {
+        // TODO: Have a better reallocation strategy
+        int new_size = bp->capacity + len + 1;
+        bp->buf = realloc(bp->buf, new_size);
+        if(bp->buf == NULL) {
+            free(bp);
+            HPyErr_NoMemory(ctx);
+            return -1;
+        }
     }
-    Py_INCREF(item);
-    PyList_SET_ITEM(bp->list, bp->length++, item);
+    strncpy((bp->buf + bp->length), s, len);
+    bp->length += len;
     return 0;
 }
 
 _HPy_HIDDEN HPy
 ctx_UnicodeBuilder_Build(HPyContext *ctx, HPyUnicodeBuilder builder)
 {
     _PyUnicodeBuilder_s *bp = _hb2pb(builder);
-    PyObject *list = PyList_GetSlice(bp->list, 0, bp->length);
-
-    PyObject *sep = PyUnicode_FromString("");
-    PyObject *str = PyUnicode_Join(sep, list);
-    Py_XDECREF(sep);
-
-    if(str == NULL) {
-        PyErr_NoMemory();
-        return HPy_NULL;
-    }
-
-    Py_XDECREF(bp->list);
-    Py_XDECREF(list);
+    HPy h_result = HPyUnicode_FromString(ctx, bp->buf);
+    free(bp->buf);
     free(bp);
-    return _py2h(str);
+    return h_result;
 }
 
 _HPy_HIDDEN void
 ctx_UnicodeBuilder_Cancel(HPyContext *ctx, HPyUnicodeBuilder builder)
 {
     _PyUnicodeBuilder_s *bp = _hb2pb(builder);
-    Py_XDECREF(bp->list);
+    free(bp->buf);
     free(bp);
 }

hpy/devel/include/cpython/hpy.h

@@ -284,7 +284,7 @@ HPy HPyListBuilder_Build(HPyContext *ctx, HPyListBuilder builder);
 void HPyListBuilder_Cancel(HPyContext *ctx, HPyListBuilder builder);
 
 HPyUnicodeBuilder HPyUnicodeBuilder_New(HPyContext *ctx, HPy_ssize_t size);
-int HPyUnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, HPy h_item);
+int HPyUnicodeBuilder_Add(HPyContext *ctx, HPyUnicodeBuilder builder, const char *s);
 HPy HPyUnicodeBuilder_Build(HPyContext *ctx, HPyUnicodeBuilder builder);
 void HPyUnicodeBuilder_Cancel(HPyContext *ctx, HPyUnicodeBuilder builder);
 

hpy/devel/include/universal/autogen_ctx.h

@@ -3,49 +3,21 @@
 class TestString(HPyTest):
     def test_unicode_builder(self):
         mod = self.make_module("""
-            HPyDef_METH(f, "f", f_impl, HPyFunc_O)
-            static HPy f_impl(HPyContext *ctx, HPy h_self, HPy h_arg)
+            HPyDef_METH(f, "f", f_impl, HPyFunc_NOARGS)
+            static HPy f_impl(HPyContext *ctx, HPy h_self)
             {
                 HPyUnicodeBuilder builder = HPyUnicodeBuilder_New(ctx, 0);
                 if(HPy_IsNull(builder)) {
                     HPyErr_SetString(ctx, ctx->h_RuntimeError, "Could not create HPyUnicodeBuilder");
                     return HPy_NULL;
                 }
-                HPy h_s1 = HPyUnicode_FromString(ctx, "hello ");
-                HPy h_s2 = HPyUnicode_FromString(ctx, "!");
-                HPyUnicodeBuilder_Add(ctx, builder, h_s1);
-                HPyUnicodeBuilder_Add(ctx, builder, h_arg);
-                HPyUnicodeBuilder_Add(ctx, builder, h_s2);
+                HPyUnicodeBuilder_Add(ctx, builder, "hello ");
+                HPyUnicodeBuilder_Add(ctx, builder, "world");
+                HPyUnicodeBuilder_Add(ctx, builder, "!");
                 HPy h_string = HPyUnicodeBuilder_Build(ctx, builder);
-                HPy_Close(ctx, h_s1);
-                HPy_Close(ctx, h_s2);
                 return h_string;
             }
             @EXPORT(f)
             @INIT
         """)
-        assert mod.f("world") == "hello world!"
-
-    def test_type_error(self):
-        mod = self.make_module("""
-            HPyDef_METH(f, "f", f_impl, HPyFunc_O)
-            static HPy f_impl(HPyContext *ctx, HPy h_self, HPy h_arg)
-            {
-                HPyUnicodeBuilder builder = HPyUnicodeBuilder_New(ctx, 0);
-                if(HPy_IsNull(builder)) {
-                    HPyErr_SetString(ctx, ctx->h_RuntimeError, "Could not create HPyUnicodeBuilder");
-                    return HPy_NULL;
-                }
-                HPy h_long = HPyLong_FromLong(ctx, 42);
-                HPyUnicodeBuilder_Add(ctx, builder, h_long);
-                HPy_Close(ctx, h_long);
-                HPyUnicodeBuilder_Cancel(ctx, builder);
-                return HPy_NULL;
-            }
-            @EXPORT(f)
-            @INIT
-        """)
-        import pytest
-        with pytest.raises(TypeError) as exc_info:
-            mod.f("world")
-        assert exc_info.match("Argument must be of type HPyUnicode")
+        assert mod.f() == "hello world!"