Is the obj-C code gen complete?

[jackpal]

I was comparing the java and objc code gen for a stored procedure. I would expect them to be similar, but the Obj-C code gen is much shorter. Is it incomplete? If so you might want to note that in the docs somewhere.

[ricomariani]

It calls the C code, so it's just extra wrapping for the result set. You need both. The Java is similarly helpers for reading the result set.

[jackpal]

But even with the generated C code, the objective-C code seems incomplete.

I did this to generate the objective C header:

cql --in todo.sql --cg todo.h todo.c
cql --in todo.sql --rt objc --objc_c_include_path todo.h --cg todo-objc.h

But the code in todo-objc.h doesn't seem to match the types in the todo.h header.

For example, the objc header is referring to class CGBTodoTasks, which is not declared or implemented anywhere, and it's referring to types like CGCTodoTasksResultSetRef that are not declared anywhere.

@class CGBTodoTasks;

static inline CGBTodoTasks *CGBTodoTasksFromCGCTodoTasks(CGCTodoTasksResultSetRef resultSet)
{
  return (__bridge CGBTodoTasks *)resultSet;
}

static inline CGCTodoTasksResultSetRef CGCTodoTasksFromCGBTodoTasks(CGBTodoTasks *resultSet)
{
  return (__bridge CGCTodoTasksResultSetRef)resultSet;
}

static inline int64_t CGBTodoTasksGetRowid(CGBTodoTasks *resultSet, int32_t row)
{
  CGCTodoTasksResultSetRef cResultSet = CGCTodoTasksFromCGBTodoTasks(resultSet);
  return CGCTodoTasksGetRowid(cResultSet, row);
}

[ricomariani]

Well poop. You're obviously the first person outside of FB to try to use this.

Here's what's going on and why it's broken.

The system has the option of taking snake_case_names in the stored procs and converting them to PascalCaseNames in the output. It turns out this was a really bad idea... Anywho it can also add a prefix. You can see the options in the rt_common.c. Anyway, it turns out FB uses a prefix so that every is named the same. Let's say it's FBQ so you get names like FBQYourProcResultSetRef.

For anyone using the normal runtime options, like you, you want none of this. But the objc defaults are kicking in.

To avoid getting different test results in the internal build vs. the normal build I just normalize everything to CGCWhatever.

This turns out again to be wrong... In the normal build those should never have been CamelCased at all much less prefixed.

So while it means I have to fork some tests, the public version of objc is currently broken. Interestingly, the Java doesn't seem to have this problem. But's because it only provides a result set wrapper anyway.

It turns out all this name mangling was a huge mistake but I'm stuck with it. Two nominally smart people thought it was a good idea at the time :D

[ricomariani]

BTW the objc generation predates the JSON... if I was doing this today, I wouldn't :D -- I would say "this is an example of exactly the sort of glue you can generate yourself from the JSON and you should generate whatever classes fit nicely into your environment with whatever naming convention you like :D"

[ricomariani]

With regard to the class, IIRC it's there to do retain/release. It has no members etc so it needs no body. All the useful methods are inline functions. The real problem is that CGCTodoTasksGetRowid should be todo_tasks_get_rowid(...) or something like that.

[ricomariani]

To see where this is heading:

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

Now I have to fix the internal stuff this will break :D

[ricomariani]

You can try applying the PR locallly to see if it helps you move forward. It should at least make sense now.

[jackpal]

Thanks!! I am new to all this C-ObjC-Swift bridging stuff, so it is helpful to have the Objc code gen as a working example.

[ricomariani]

Well, it almost works... See... here's the thing. It's going to blow up as soon as you try to use a string column. Here's why:

Cqlrt.h/cqlrt.c is intended to be replaced. The OSS version is simple/portable/single-threaded. It has basically zero dependencies other than things like strcpy and malloc. The production version of cqlrt is for iOS mainly and it uses the CF types. So cql_string_ref for instance becomes basically CFString. Which means it can be bridged. The default cql_string_ref cannot be bridged!

So to use the objc output you really want to have an objc-friendly version of cqlrt.c

It turns out this is pretty easy, so I was thinking what I would do is make a simple version say cqlrt_cf.c and cqlrt_cf.h that goes with it. Then you switch that in using the --cqlrt option and you're off to the races.

There are at least 3 cqlrt.c flavors that I'm aware of including the OSS one, this would be a fourth.

So things like:

typedef struct cql_string *cql_string_ref;

become:

#define cql_string_ref CFString

All of the hard stuff is done in cqlrt_common.c which is expecting this sort of thing.

[ricomariani]

See also: https://cgsql.dev/cql-guide/int05

[ricomariani]

If you need "cqlrt_cf" to continue your experiments I can probably bust it out today but I'd rather fix some of those doc issues first. I suspect when I audit objc code gen I will find a few other lingering things that I didn't already fix. The _cf version of cqlrt will help flush those out. In some sense the core is well tested in that FB uses it all over the place but there are sneak assumptions in places that can be hard for me to see because I'm blind to my own assumptions.

[jackpal]

Wow, thanks for the info, that explains a lot.

I would love to see a cqlrt_cf, but I totally understand if you have other work. (I am working on this for fun, and next week I have to go back to work, so having something this week is better for me than next week, but I should still have time even if it's next week.)

By the way, how does ref counting work with obj-c class that wraps the result_set_ref? (The one that's called CGBTodoTasks in the current version of the obj-c generated code?

I was expecting the generated code for CGBTodoTasks to have a dealloc method that called cql_result_set_release.

[ricomariani]

You just use the cql_release method which cqlrt.h defines to by definition do whatever the right thing is.

---

From: Jack Palevich @.> Sent: Tuesday, December 28, 2021 2:14 PM To: facebookincubator/CG-SQL @.> Cc: Rico Mariani @.>; Comment @.> Subject: Re: [facebookincubator/CG-SQL] Is the obj-C code gen complete? (Issue #99)

Wow, thanks for the info, that explains a lot.

I would love to see a cqlrt_cf, but I totally understand if you have other work. (I am working on this for fun, and next week I have to go back to work, so having something this week is better for me than next week, but I should still have time even if it's next week.)

By the way, how does ref counting work with obj-c class that wraps the result_set_ref? (The one that's called CGBTodoTasks in the current version of the obj-c generated code?

I was expecting the generated code for CGBTodoTasks to have a dealloc method that called cql_result_set_release.

— Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ffacebookincubator%2FCG-SQL%2Fissues%2F99%23issuecomment-1002302433&data=04%7C01%7C%7C8e845ce0d41843a9478d08d9ca4f6c9b%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637763264743294160%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=EDu3FkxdSlp%2F2qJE6mLg4MIwH1DuV6OcCWlIS06IxjM%3D&reserved=0, or unsubscribehttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAHC3V72UEHHAXDVOABX7F6LUTIZEPANCNFSM5K3HYQSA&data=04%7C01%7C%7C8e845ce0d41843a9478d08d9ca4f6c9b%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637763264743294160%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=VLtKFAi49Q8qmxSTtlLmZbk68%2Fw7oN4YSF5h2btMdy8%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%7C8e845ce0d41843a9478d08d9ca4f6c9b%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637763264743294160%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=Sd3%2BlDEWv6ZgHMnAMRMckYtSH3gFPZ16yL2rITUzgHU%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%7C8e845ce0d41843a9478d08d9ca4f6c9b%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637763264743294160%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=E%2FtmCWuEOlXh3ns2q%2BHJqsj74a8sPjxehj%2Bs%2FlwcPJ4%3D&reserved=0. You are receiving this because you commented.Message ID: @.***>

[jackpal]

OK -- I guess that means that I need to track the lifetime of the result set, rather than letting the Obj-C runtime do it.

[jackpal]

FWIW, I am writing a Swift Package by hand that creates a hand-rolled Swift interface for the todo toy project. This seems to be working pretty well (and should in theory even work on Linux...) I have unit tests as part of the package, which are helpful for development.

I originally hoped that the C and Obj-C headers would be enough for a Swift API, but it looks like Swift's FFI doesn't handle some of the techniques that cgl uses in the generated headers.

For example, Swift can't call any #defines, so it can't call cql_result_set_release. I have to either write Obj-c wrappers or use the underlying cql_release method.

I have my toy app's non-result-set methods (create/insert/delete) working, now I'm working on the result set.

There are a lot of ways the Swift API might look -- lots of difference Swift idioms. (For example using "try" instead of error return codes.)

That brings up a question: how is a large project supposed to be organized with cql? Should I have one Swift Package for each cql compilation unit, or should I group all of the cql compilation units together into one Swift Package? I guess part of the question is whether the runtime files can be duplicated multiple times in the same app.

Do you have any advice?

[ricomariani]

Well, there's no law that says cqlrt has to define those helper functions with macros. They are just function-like syntax in C. You can include any number of useful helper function in your cqlrt.c that Swift can use. You can even include more swift-friendly versions that map down to the bare bones version. All of this designed to be replaced so if you want to include your own cql_result_set_release_for_swift you can, it does the conversions. Our iOS cql runtime looks nothing like the default one other than the base names are the same. All the workers are different. It even has statement caching which the original does not, but the hooks are there to do your own.

How you group your procedures is again up to you. We ended up having "bundles" of procedures so you basically put a bunch of files in one directory and they are compiled together and you make dependencies between bundles. CQL doesn't know this is going on, for us it happens with our build tool BUCK. You could do the same, or you could do one per file, or you could just have one mondo file. We have bundles because there are lots of devs and you don't want to have conflicts all the the time. But the bundles are discoverable so when you need to know all the dependencies you can make a big mondo JSON for the union of them all. That's our compromise.

Really for these kinds of questions I tend to answer "clang is silent on such things, so is CQL" :D

[jackpal]

Makes sense. I was thinking cqlrt.[ch] was provided by cg-sql, but I see now that it's semantically provided by the client, and you just happen to provide an example.

[ricomariani]

Indeed. The one I provide is not especially good either :D

[ricomariani]

We provide cqlrt_common.c and cqlrt_common.h :D

[ricomariani]

I made a dent in cqlrt_cf but it's not ready to ship yet. I'll have to see what I can do tomorrow. I do want to fix these doc issues. There were a bunch and they are super easy to address.

[jackpal]

I got my hand-crafted Swift Package for the todo app limping along. I didn't need to use the obj-c RT, but I did use the C RT.

The API currently looks like this:

for task in try TodoTasks(db:db) {
    print("\(task.row): \(task.txt) rowID: \(task.rowID) done: \(task.done)")
}

I still need to figure out how to make a change-observing ObservableObject wrapper for use with SwiftUI.

Now to write a tool to generate this code automatically from the json metadata.

[ricomariani]

Cool. The objc fixes are in the process of landing now. The cqlrt_cf version will probably land tomorrow with a little demo script like the java thing. It could use some docs too… it’s nearly done.

Sent from Mailhttps://go.microsoft.com/fwlink/?LinkId=550986 for Windows

---

From: Jack Palevich @.> Sent: Tuesday, December 28, 2021 8:25:41 PM To: facebookincubator/CG-SQL @.> Cc: Rico Mariani @.>; Comment @.> Subject: Re: [facebookincubator/CG-SQL] Is the obj-C code gen complete? (Issue #99)

I got my hand-crafted Swift Package for the todo app limping along. I didn't need to use the obj-c RT, but I did use the C RT.

The API currently looks like this:

for task in try TodoTasks(db:db) { print("(task.row): (task.txt) rowID: (task.rowID) done: (task.done)") }

I still need to figure out how to make a change-observing ObservableObject wrapper for use with SwiftUI.

Now to write a tool to generate this code automatically from the json metadata.

— Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ffacebookincubator%2FCG-SQL%2Fissues%2F99%23issuecomment-1002391731&data=04%7C01%7C%7C3146cfb24c514b54960808d9ca834698%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637763487439737673%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=g%2FeLo8qGHdj6X%2FbvTbdWj%2BWpnZiRBZNAZ7AP523tA%2BQ%3D&reserved=0, or unsubscribehttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAHC3V7Y4BYA5AVOECHOFCPTUTKEULANCNFSM5K3HYQSA&data=04%7C01%7C%7C3146cfb24c514b54960808d9ca834698%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637763487439737673%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=8wCvKyTuFPeeW%2FlhlnlxzQUCG5gaY6Ow1AdqjyXRR3Y%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%7C3146cfb24c514b54960808d9ca834698%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637763487439737673%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=kxIB2Yf5a0HjVG0pEeqk3O%2Fey6Vh3IvUPL%2B7P8upy68%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%7C3146cfb24c514b54960808d9ca834698%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637763487439893873%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=N4GfiSHRWA%2FJ4bOPfoHmLttxdI%2BtPgnOxG93ZXfIAb8%3D&reserved=0. You are receiving this because you commented.Message ID: @.***>

[ricomariani]

cqlrt_cf is ready to ship, it's in code review now. It came out pretty nice.

[ricomariani]

As you can see above: https://github.com/facebookincubator/CG-SQL/commit/b2e1aa6375c7c03b04a591f92e1977f434a080e3 has landed!

This includes your suggested todo demo code (now in appendix 10) ported to a .m file and using types like NSString. It writes with NSLog. The result set interface is still "C" like but that was how it always was. The important thing is that the types that flow are CF types so they can be bridged easily.

Note that cqlrt_cf is only lightly tested so if you want to play with it some for your use-case that would be great. I'd be happy to take bug reports.

[ricomariani]

Feel free to close this issue if you deem it closed. I'll close it later today after I give you a chance to tell me why I am idiot and it's not ready to be closed :D

[ricomariani]

I think we're good to go :D

[jackpal]

This looks great! Thank you!

The only thing I can think to suggest is to add a .gitignore in sources/cqlrt_cf/.gitignore, something like

./demo
./demo.dSYM/
./demo_objc.h
./demo_todo.c
./demo_todo.h

[ricomariani]

Sure easy enough.

---

From: Jack Palevich @.> Sent: Thursday, December 30, 2021 9:59 PM To: facebookincubator/CG-SQL @.> Cc: Rico Mariani @.>; State change @.> Subject: Re: [facebookincubator/CG-SQL] Is the obj-C code gen complete? (Issue #99)

This looks great! Thank you!

The only thing I can think to suggest is to add a .gitignore in sources/cqlrt_cf/.gitignore, something like

./demo ./demo.dSYM/ ./demo_objc.h ./demo_todo.c ./demo_todo.h

— Reply to this email directly, view it on GitHubhttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ffacebookincubator%2FCG-SQL%2Fissues%2F99%23issuecomment-1003278820&data=04%7C01%7C%7C2e7c5b80fe00437d422408d9cc22ab96%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637765271547658444%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=0pi1siFPVBb0zgLLvLnuOU2OKabzSsOGxBBtAL1%2Bzk4%3D&reserved=0, or unsubscribehttps://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAHC3V7ZEUN2357CSTGEHYS3UTVBC7ANCNFSM5K3HYQSA&data=04%7C01%7C%7C2e7c5b80fe00437d422408d9cc22ab96%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637765271547658444%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=2yu5MQu25dby8slKLibur2UxwOKOSY6yDMJHMC4HQ9w%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%7C2e7c5b80fe00437d422408d9cc22ab96%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637765271547658444%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=wrORyw%2Br6J55d3Kmih%2BjzpKVc%2FCYrtDkRcybz7NoeHs%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%7C2e7c5b80fe00437d422408d9cc22ab96%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637765271547658444%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=KmvqCbzcO%2FhBrVuJOSvB%2FtonF%2F%2BVyP%2FOL4Qy0sN75LE%3D&reserved=0. You are receiving this because you modified the open/close state.Message ID: @.***>