Exported generators under -shared: print boxed objects and quote internal field names

tslang/include/TypeScript/MLIRLogic/MLIRPrinter.h

@@ -9,6 +9,7 @@
 
 #include "llvm/Support/Debug.h"
 #include "llvm/ADT/APSInt.h"
+#include "llvm/ADT/StringExtras.h"
 
 #include <functional>
 
@@ -23,6 +24,39 @@ class MLIRPrinter
 {
   public:
 
+    // print an object held by reference (ObjectType over a tuple, e.g. a generator's object)
+    // as `BoxedObject<{...}>` rather than as its bare shape `{...}`, which reads back as a
+    // value tuple. Declaration text (__decls) needs it: an importer that took the object for a
+    // value would disagree with the library on its layout. Off for diagnostics.
+    bool printBoxedObjectTypes = false;
+
+    // print a field name that is not an identifier (an internal one such as a generator's
+    // `.step`) quoted, `".step": s32`: unquoted it is a syntax error, and the importer dropped
+    // the whole declaration. Off for diagnostics.
+    bool quoteNonIdentifierFieldNames = false;
+
+    template <typename T>
+    void printFieldName(T &out, mlir::Attribute id)
+    {
+        if (quoteNonIdentifierFieldNames)
+        {
+            if (auto strAttr = dyn_cast<mlir::StringAttr>(id))
+            {
+                auto name = strAttr.getValue();
+                auto isIdentifierChar = [](char c) { return llvm::isAlnum(c) || c == '_' || c == '$'; };
+                if (name.empty() || llvm::isDigit(name.front()) || !llvm::all_of(name, isIdentifierChar))
+                {
+                    out << "\"";
+                    out.write_escaped(name);
+                    out << "\"";
+                    return;
+                }
+            }
+        }
+
+        printAttribute(out, id, true);
+    }
+
     template <typename T, typename F>
     void printFuncType(T &out, F t)
     {
@@ -185,15 +219,15 @@ class MLIRPrinter
             // silently misaligns every subsequent read through the field.
             if (allowMethodSignature && field.id && isa<mlir_ts::FunctionType>(field.type))
             {
-                printAttribute(out, field.id, true);
+                printFieldName(out, field.id);
                 printFuncTypeAsMethodSignature(out, mlir::cast<mlir_ts::FunctionType>(field.type));
                 first = false;
                 continue;
             }
 
             if (field.id)
             {
-                printAttribute(out, field.id, true);
+                printFieldName(out, field.id);
                 out << ":";
             }
 
@@ -448,6 +482,13 @@ class MLIRPrinter
                 // into a real structural type on reimport instead of degrading to
                 // bare `object` (which has no fields/methods to cast against).
                 auto storageType = t.getStorageType();
+                auto isBoxedShape = isa<mlir_ts::TupleType>(storageType) || isa<mlir_ts::ConstTupleType>(storageType)
+                    || isa<mlir_ts::ObjectStorageType>(storageType);
+                if (isBoxedShape && printBoxedObjectTypes)
+                {
+                    out << "BoxedObject<";
+                }
+
                 if (auto tupleType = dyn_cast<mlir_ts::TupleType>(storageType))
                 {
                     printObjectType(out, tupleType);
@@ -464,6 +505,11 @@ class MLIRPrinter
                 {
                     out << "object";
                 }
+
+                if (isBoxedShape && printBoxedObjectTypes)
+                {
+                    out << ">";
+                }
             })
             .template Case<mlir_ts::ObjectStorageType>([&](auto t) {
                 printObjectType(out, t);

tslang/lib/TypeScript/DeclarationPrinter.cpp

@@ -24,6 +24,8 @@ namespace typescript
     void MLIRDeclarationPrinter::print(mlir::Type type)
     {
         MLIRPrinter mp{};
+        mp.printBoxedObjectTypes = true;
+        mp.quoteNonIdentifierFieldNames = true;
         mp.printType<raw_ostream>(os, type);
     }
 
@@ -361,6 +363,10 @@ namespace typescript
         // ordinary `import '...'` - see mlirGenImportSharedLib's '.' hack)
         // knows to dereference one extra level instead of reading the tuple
         // inline at the resolved symbol address.
+        // The variable's own type is then printed as the bare shape - @boxed already adds the
+        // extra dereference - while objects held by reference inside it (a generator returned
+        // by a method, say) still print as BoxedObject<...>.
+        auto printedType = type;
         if (auto objectType = dyn_cast<mlir_ts::ObjectType>(type))
         {
             auto storageType = objectType.getStorageType();
@@ -369,11 +375,12 @@ namespace typescript
             {
                 os << "@boxed";
                 newline();
+                printedType = storageType;
             }
         }
 
         os << (isConst ? "const" : "let") << " " << name << " : ";
-        print(type);
+        print(printedType);
         os << ";";
         newline();
 

tslang/lib/TypeScript/MLIRGenTypes.cpp

@@ -903,6 +903,7 @@ namespace mlirgen
             {"Opaque", true }, // to support void*
             {"Reference", true }, // to support dll import
             {"Ref", true }, // alias of Reference
+            {"BoxedObject", true }, // an object held by reference, see MLIRPrinter::printBoxedObjectTypes
             {"Readonly", true },
             {"Partial", true },
             {"Required", true },
@@ -991,6 +992,7 @@ namespace mlirgen
             {"Opaque", true }, // to support void*
             {"Reference", true }, // to support dll import
             {"Ref", true }, // alias of Reference
+            {"BoxedObject", true }, // an object held by reference, see MLIRPrinter::printBoxedObjectTypes
             {"ThisType", true },
             //{"Array", true }
         };
@@ -1144,13 +1146,14 @@ namespace mlirgen
     {
         enum class EmbeddedType
         {
-            None, TypeOf, Reference, FirstTypeArgument, NonNullable, Array, ReadonlyArray, ReturnType,
+            None, TypeOf, Reference, BoxedObject, FirstTypeArgument, NonNullable, Array, ReadonlyArray, ReturnType,
             Parameters, ThisParameterType, OmitThisParameter, Uppercase, Lowercase, Capitalize, Uncapitalize
         };
 
         auto kind = llvm::StringSwitch<EmbeddedType>(name)
             .Case("TypeOf", EmbeddedType::TypeOf)
             .Cases("Reference", "Ref", EmbeddedType::Reference)
+            .Case("BoxedObject", EmbeddedType::BoxedObject)
             .Cases("Readonly", "Partial", "Required", "ThisType", EmbeddedType::FirstTypeArgument)
             .Case("NonNullable", EmbeddedType::NonNullable)
 #ifdef ARRAY_TYPE_AS_ARRAY_CLASS
@@ -1184,6 +1187,8 @@ namespace mlirgen
                 return mth.wideStorageType(type);
             case EmbeddedType::Reference:
                 return mlir_ts::RefType::get(type);
+            case EmbeddedType::BoxedObject:
+                return getObjectType(type);
             case EmbeddedType::FirstTypeArgument:
                 return type;
             case EmbeddedType::NonNullable:
@@ -1240,12 +1245,13 @@ namespace mlirgen
     {
         enum class EmbeddedType
         {
-            None, TypeOf, Reference, ThisType, Array
+            None, TypeOf, Reference, BoxedObject, ThisType, Array
         };
 
         auto kind = llvm::StringSwitch<EmbeddedType>(name)
             .Case("TypeOf", EmbeddedType::TypeOf)
             .Cases("Reference", "Ref", EmbeddedType::Reference)
+            .Case("BoxedObject", EmbeddedType::BoxedObject)
             .Case("ThisType", EmbeddedType::ThisType)
 #ifdef ARRAY_TYPE_AS_ARRAY_CLASS
             .Case("Array", EmbeddedType::Array)
@@ -1264,6 +1270,8 @@ namespace mlirgen
                 return mth.wideStorageType(type);
             case EmbeddedType::Reference:
                 return mlir_ts::RefType::get(type);
+            case EmbeddedType::BoxedObject:
+                return getObjectType(type);
             case EmbeddedType::ThisType:
                 return type;
 #ifdef ARRAY_TYPE_AS_ARRAY_CLASS

tslang/test/tester/CMakeLists.txt

@@ -977,6 +977,7 @@ add_test(NAME test-compile-export-import-class-interface COMMAND test-runner "${
 add_test(NAME test-compile-export-import-owned-returns COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_owned_returns.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_owned_returns.ts")
 add_test(NAME test-compile-export-import-const-locals COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_const_locals.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_const_locals.ts")
 add_test(NAME test-compile-export-import-const-functions COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_const_functions.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_const_functions.ts")
+add_test(NAME test-compile-export-import-generators COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_generators.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_generators.ts")
 add_test(NAME test-compile-rc-export-import-owned-returns COMMAND test-runner -mm=rc "${PROJECT_SOURCE_DIR}/test/tester/tests/import_owned_returns.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_owned_returns.ts")
 add_test(NAME test-compile-none-export-import-owned-returns COMMAND test-runner -mm=none "${PROJECT_SOURCE_DIR}/test/tester/tests/import_owned_returns.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_owned_returns.ts")
 add_test(NAME test-compile-export-import-class-extends COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends.ts")
@@ -1062,6 +1063,7 @@ add_test(NAME test-compile-shared-export-import-owned-returns COMMAND test-runne
 add_test(NAME test-compile-shared-export-import-gc-single-collector COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_gc_single_collector.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_gc_single_collector.ts")
 add_test(NAME test-compile-shared-export-import-const-locals COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_const_locals.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_const_locals.ts")
 add_test(NAME test-compile-shared-export-import-const-functions COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_const_functions.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_const_functions.ts")
+add_test(NAME test-compile-shared-export-import-generators COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_generators.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_generators.ts")
 add_test(NAME test-compile-shared-export-import-class-extends COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends.ts")
 add_test(NAME test-compile-shared-export-import-class-extends-implements-diamond COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_implements_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_implements_diamond.ts")
 add_test(NAME test-compile-shared-export-import-class-extends-multilevel COMMAND test-runner -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_multilevel.ts")
@@ -1146,6 +1148,7 @@ add_test(NAME test-jit-shared-export-import-owned-returns COMMAND test-runner -j
 add_test(NAME test-jit-shared-export-import-gc-single-collector COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_gc_single_collector.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_gc_single_collector.ts")
 add_test(NAME test-jit-shared-export-import-const-locals COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_const_locals.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_const_locals.ts")
 add_test(NAME test-jit-shared-export-import-const-functions COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_const_functions.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_const_functions.ts")
+add_test(NAME test-jit-shared-export-import-generators COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_generators.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_generators.ts")
 add_test(NAME test-jit-shared-export-import-class-extends COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends.ts")
 add_test(NAME test-jit-shared-export-import-class-extends-multilevel COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_multilevel.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_multilevel.ts")
 add_test(NAME test-jit-shared-export-import-class-extends-implements-diamond COMMAND test-runner -jit -shared "${PROJECT_SOURCE_DIR}/test/tester/tests/import_class_extends_implements_diamond.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/export_class_extends_implements_diamond.ts")

tslang/test/tester/tests/export_generators.ts

@@ -0,0 +1,27 @@
+// The library side of import_generators.ts. A generator returns its object by reference; the
+// declaration text importers read (__decls) used to print that object as a plain `{...}`, which
+// reads back as a value, so a -shared importer got garbage out of next().
+
+export function* functionGenerator() {
+    yield 1;
+    yield 2;
+}
+
+export const constGenerator = function* () {
+    yield 3;
+    yield 4;
+};
+
+export class WithGenerator {
+    *items() {
+        yield 5;
+        yield 6;
+    }
+}
+
+namespace NS {
+    export function* inNamespace() {
+        yield 7;
+        yield 8;
+    }
+}

tslang/test/tester/tests/import_generators.ts

@@ -0,0 +1,28 @@
+import './export_generators'
+
+// Iterating generators exported from another module. See export_generators.ts.
+
+function main() {
+    let a = 0;
+    for (const v of functionGenerator()) a += v;
+    assert(a == 3, "functionGenerator");
+
+    let b = 0;
+    for (const v of constGenerator()) b += v;
+    assert(b == 7, "constGenerator");
+
+    let c = 0;
+    for (const v of new WithGenerator().items()) c += v;
+    assert(c == 11, "WithGenerator.items");
+
+    let d = 0;
+    for (const v of NS.inNamespace()) d += v;
+    assert(d == 15, "NS.inNamespace");
+
+    const g = functionGenerator();
+    assert(g.next().value == 1, "next 1");
+    assert(g.next().value == 2, "next 2");
+    assert(g.next().done, "next done");
+
+    print("done.");
+}

tslang/unittests/MLIRGen/DeclarationPrinter.cpp

@@ -758,6 +758,40 @@ TEST_F(DeclarationPrinterTest, variable_of_object_with_non_tuple_storage_has_no_
     EXPECT_THAT(text, testing::Not(testing::HasSubstr("@boxed")));
 }
 
+TEST_F(DeclarationPrinterTest, boxed_variable_prints_its_own_type_as_bare_shape)
+{
+    // @boxed already tells the importer to dereference once more; wrapping the type in
+    // BoxedObject<...> too would make it dereference twice.
+    llvm::SmallVector<mlir_ts::FieldInfo> fields{classField(strAttr("x"), get<mlir_ts::NumberType>())};
+    auto objType = mlir_ts::ObjectType::get(getContext(), mlir_ts::TupleType::get(getContext(), fields));
+    auto text = printed([&](MLIRDeclarationPrinter &dp) { dp.printVariableDeclaration("obj", noNamespace(), objType, true); });
+    EXPECT_THAT(text, testing::HasSubstr("const obj : {x:number};"));
+    EXPECT_THAT(text, testing::Not(testing::HasSubstr("BoxedObject")));
+}
+
+TEST_F(DeclarationPrinterTest, function_returning_object_held_by_reference_prints_boxed_object)
+{
+    // a generator function returns its object by reference (ObjectType over a tuple); printed
+    // as a bare `{...}` the importer read it back as a value tuple and the layouts disagreed.
+    llvm::SmallVector<mlir_ts::FieldInfo> fields{classField(strAttr("x"), get<mlir_ts::NumberType>())};
+    auto objType = mlir_ts::ObjectType::get(getContext(), mlir_ts::TupleType::get(getContext(), fields));
+    auto text = printed([&](MLIRDeclarationPrinter &dp) { dp.print("gen", noNamespace(), getF({}, {objType})); });
+    EXPECT_THAT(text, testing::HasSubstr("BoxedObject<{x:number}>"));
+}
+
+TEST_F(DeclarationPrinterTest, field_name_that_is_not_an_identifier_is_quoted)
+{
+    // a generator object's internal `.step` field: unquoted it is a syntax error and the
+    // importer dropped the whole declaration.
+    llvm::SmallVector<mlir_ts::FieldInfo> fields{
+        classField(strAttr(".step"), get<mlir_ts::NumberType>()),
+        classField(strAttr("value"), get<mlir_ts::NumberType>())};
+    auto text = printed([&](MLIRDeclarationPrinter &dp) {
+        dp.printTypeDeclaration("T", noNamespace(), mlir_ts::TupleType::get(getContext(), fields));
+    });
+    EXPECT_THAT(text, testing::HasSubstr("{\".step\":number, value:number}"));
+}
+
 // ---------------------------------------------------------------------------
 // Function printing: params (with optional-param and this-elision handling
 // shared with printMethod via printParams), and return-type presence.