Swift state of the world: what's left to be more swift friendly

[ricomariani]

From @jackpal

--- quoted comment begins

Sorry for not being clear. Here's what currently remains in my swifthelpers.h file:

pragma once

#import "todo_objc.h"

extern CQL_WARN_UNUSED cql_code all_tasks(sqlite3 *_Nonnull _db_, sqlite3_stmt *_Nullable *_Nonnull _result_stmt);

@interface CGS_all_tasks
@end

static inline cql_int32 get_all_tasks_data_types_count() { return all_tasks_data_types_count; }
static inline cql_int64 get_CRC_all_tasks() { return CRC_all_tasks; };

There are 4 items here.

Category 1: critical stuff that is required for Swift Interop:

@interface CGS_all_tasks
@end

This is required, otherwise Swift doesn't import the CGS_all_tasks class.

Category 2: Stuff that's required for less important features

static inline cql_int64 get_CRC_all_tasks() { return CRC_all_tasks; };

The CRC is not that important for toy apps, but this is the only way to make it available to Swift.

Category 3: Stuff that can be synthesized by the Swift generator from the json file

static inline cql_int32 get_all_tasks_data_types_count() { return all_tasks_data_types_count; }

As far as I can tell, this is just "number of columns in the result set", but maybe I am wrong.

Category 4: Experimental stuff.

extern CQL_WARN_UNUSED cql_code all_tasks(sqlite3 *_Nonnull _db_, sqlite3_stmt *_Nullable *_Nonnull _result_stmt);

This is something I think you ought to expose in the C and Objective-C APIs, but you don't currently, so there's no reason to expect you to add it just for Swift.

[jackpal]

Oh, err, I guess technically categories 2,3,4 are stuff from the C runtime that should be exposed.

[ricomariani]

Type 1

Can do with an #ifdef or something. The thing is we don't normally want to pay the cost of the interface metadata but we could make it so you can opt-in to the interface.

Type 2

Easy enough to add, not really super helpful but won't really cost anything anyway.

Type 3

This gives you the number of columns in the result set, which I guess is useful

Type 4

I've shied away from this because it's not at all typesafe. You have to "know" the column types and order and etc to use this safely. You can of course call it but I've made it a bit more difficult on purpose. The idea is that only something that is doing its own statement wrapping should use the thing and for such a piece of code.

Summary:

1: seems totally necessary

2: super easy, no reason not to

3: of dubious value, without #4, but doesn't cause any problems

4: I'd rather not given that there's heavy lifting to be had to use this anyway and for type safety reasons I wouldn't want to expose this for incidental use.

How do you feel about that?

[ricomariani]

BTW the reason those methods are public is that CQL cross procedure calls use that contract to get cursors and such like. But of course they know the the result shape from the procedure declarations.

[jackpal]

I understand your reasoning, and it's pretty much what I expected.

I do want to defend exposing the prepared statement. As far as I can tell, the prepared statement has enough sqlite3 metadata that it's useful by itself, without having to know anything about the cg-sql runtime. The prepared statement has the big benefit over the wrapped version of materializing the result incrementally.

[ricomariani]

I totally agree about the benefits, this is why we CQL uses that contract itself. I guess another option here is to opt into exposing it via an ifdef... or a compiler flag. I feel like every time compromise on type safety I regret it because some dumb thing happens :D

Let's take care of the basics, that stuff is easy and necessary.

[jackpal]

I can't think of a use for "number of columns in the result set" at the Swift level, because there's no way of using that info to call any API. So you can leave it out.

[jackpal]

As for the CRC, I am not sure if it is useful at the Swift level. There's class.type or class.self that I think serves a similar purpose. So the main benefit of exposing the CRC would be interop with non-public features of the C runtime. I think it's probably best to leave it out for now, too.

[jackpal]

You can tell I wrote the Swift code gen blindly, just porting all the things.... At least I didn't spend much time on these two items.

[ricomariani]

The CRC exists for use in profiling, so you can log the CRC of the procedure name rather than the name itself to save space and then you can lash them up later. So really, it's only useful for the C profiling hook and it already gets it as an arg. The #define is just there for compile-time wiring.

[ricomariani]

ok so just #1 for now. And we'll think on #4...

[jackpal]

sgtm

[jackpal]

A cheesy way of doing #1 if you can't figure out any better way is to have a seperate foo-Swift.h file that defines the interface and then #imports the foo-objc.h header...

[ricomariani]

#ifdef CQL_EMIT_OBJC_INTERFACES
@interface CGBComplexReturn;
@end
#endif

Then use -DCQL_EMIT_OBJC_INTERFACES

I was planning on the above

[jackpal]

Oh, I guess that would work fine.

[ricomariani]

The idea is you pay the cost of the interfaces if (and only if) you want them.

[ricomariani]

Also, swift could take the position that all result sets are the same underlying type (which they are) and have no per result set type at all, or define its own that simply holds a NSObject or something. I'm not saying that's a good idea but you could do it :D. Well I guess the problem with that is then you don't get the helper functions and such because the type is invisible to swift and then you'd have to generate all of that too, which you could but at that point it's just getting dumb :D

[ricomariani]

https://github.com/facebookincubator/CG-SQL/pull/111

[jackpal]

It just occurred to me that if you implement #109 and #110 , then Swift code can call the Obj C code without any additional generated Swift code. (It's not idiomatic Swift, but neither is sqlite3, so not that big a deal.) Some users might prefer that.

[jackpal]

The Swift way of wrapping a result set is probably something like https://developer.apple.com/documentation/tabulardata/dataframe

crossed with the way SwiftUI tuple views work. I think you would like to have a type system that lets you define Row, but Swift doesn't let you have a varying number of types in a template. So the best you can do is Row, Row<C1,C2>, Row<C1,C2,C3> until you get tired of enumerating columns, and an AnyRow() that has an array of AnyColumns.

[ricomariani]

I believe this is done now.

[jackpal]

Yes, it works. I synced with HEAD and updated my generator, and all my tests pass.

The main change was adding the -DCQL_EMIT_OBJC_INTERFACES flag to all the C and Swift targets. For the Swift Package manager, that means adding the following parameter to each target and each "testTarget" in Package.swift:

            cSettings: [.define("CQL_EMIT_OBJC_INTERFACES")],

I verified that with these changes, the Swift wrapper code works without having to generate any additional .c or .h files.

[jackpal]

For what it is worth, I have published my Python-based Swift wrapper code generator for CG-SQL:

https://github.com/jackpal/swiftgen4cgsql

I think it turned out pretty nicely. If you have any comments or suggestions I'd love to hear them.

You will notice that I did not expose the sqlite statement level result set APIs after all. I decided to hide them because they weren't compatible with UNION OUT procs.

[ricomariani]

Out and out union are super flexible but they add another set of complexities.... internally too.

I'll have a look at this tomorrow, I'm fried today but I'm super excited to check it out.

I will also be doing the other thing you suggested probably tomorrow. It's not very hard.

---

From: Jack Palevich @.> Sent: Monday, January 10, 2022 2:22 PM To: facebookincubator/CG-SQL @.> Cc: Rico Mariani @.>; Author @.> Subject: Re: [facebookincubator/CG-SQL] Swift state of the world: what's left to be more swift friendly (Issue #109)

For what it is worth, I have published my Python-based Swift wrapper code generator for CG-SQL:

https://github.com/jackpal/swiftgen4cgsqlhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjackpal%2Fswiftgen4cgsql&data=04%7C01%7C%7C9b59373d8c9746028a6508d9d487a74a%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637774501389100329%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=62VlCK%2BpLA7Cls9mDy6vru8oUgmniijsAR0TJBFDTaI%3D&reserved=0

I think it turned out pretty nicely. If you have any comments or suggestions I'd love to hear them.

You will notice that I did not expose the sqlite statement level result set APIs after all. I decided to hide them because they weren't compatible with UNION OUT procs.

— Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ffacebookincubator%2FCG-SQL%2Fissues%2F109%23issuecomment-1009401119&data=04%7C01%7C%7C9b59373d8c9746028a6508d9d487a74a%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637774501389100329%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=eu278jqdHA8IqjzTTsyvYlyzCPLUIlJf6urPVoDIfEk%3D&reserved=0, or unsubscribehttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAHC3V74GWEEOO25IRMHS4VDUVNLZLANCNFSM5LLDBXJQ&data=04%7C01%7C%7C9b59373d8c9746028a6508d9d487a74a%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637774501389110284%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=aMROrUqMn4CgNyu8bDvK2Tv6Ujxipphs4w9W6FvlcbI%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7C%7C9b59373d8c9746028a6508d9d487a74a%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637774501389110284%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=FyYnf4cp1v3P5h%2B81oGR208ktgsid%2BPmkXye9dPRrF8%3D&reserved=0 or Androidhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7C%7C9b59373d8c9746028a6508d9d487a74a%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637774501389120244%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=9ybYhl5GaG5ge62i8YiPvBvjfVkqvvZVzOyg92w3Y9U%3D&reserved=0. You are receiving this because you authored the thread.Message ID: @.***>

[ricomariani]

https://github.com/facebookincubator/CG-SQL/issues/110 has a fix ready for review which will hopefully land soon.

Everything else here is addressed so closing. :D