Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Swift] Optional Scalars Preparation #6028

Merged
merged 3 commits into from
Jul 20, 2020

Conversation

mustiikhalil
Copy link
Collaborator

@mustiikhalil mustiikhalil commented Jul 13, 2020

The following PR prepares the swift implementation to take optional scalars as a parameter. it adds two functions that swift users would find useful.

1- Optional scalars #6014

static func addInt(e: Int?) { builder.add(element: e, offset: VOffset.int.v) }
// which can be called by the generated code
SomeTable.addInt(e: nil)

2- Optional strings

// In the current swift implementation the following is required to add `String` optional values.
var offset: Offset<String>
if let e = someString {
   offset = builder.create(string: e)
} else {
  offset = Offset<String>()
}

// This is a helper method for swift since we would usually write the following:
var offset = builder.create(string: someString) // where someString can have a value or be simply nil

3- updates the version to 0.7.0 since adding optionals is a major update.

4- disable linters in the generated code

@aardappel what do you think? would an optional string helper function be useful in other languages too? should I add it?

@aardappel
Copy link
Collaborator

In languages where a string can legally be null, return a 0 offset would be a good idea that can be part of the regular CreateString. This is not the case in all languages, for example a C++ std::string cannot be null.

Not sure about the optional ints, maybe wait until the first flatc for that has landed?

@mustiikhalil
Copy link
Collaborator Author

mustiikhalil commented Jul 13, 2020

Yeah, that makes complete sense, JavaScript, maybe also csharp, and go. After all this is a nice to have.

Yes, this is just a draft for now until flatc supports it, so we can simple write the generator code for swift later on. Which would be cpp related and not swift.

@mustiikhalil mustiikhalil marked this pull request as ready for review July 20, 2020 15:25
@mustiikhalil
Copy link
Collaborator Author

@aardappel I've moved this to ready to merge incase we would want to merge this PR for now, and then start working on the cpp generator to work with optional values

@aardappel
Copy link
Collaborator

Ok, you can merge this now. Though would be good in the future to keep unrelated topics in separate PRs (small unrelated fixes / improvements are ok, but larger ones in their own PR I'd say).

@mustiikhalil
Copy link
Collaborator Author

Noted

@mustiikhalil mustiikhalil merged commit ff1b731 into google:master Jul 20, 2020
@mustiikhalil mustiikhalil deleted the optional-scalars branch July 20, 2020 17:42
mustiikhalil pushed a commit to mustiikhalil/flatbuffers that referenced this pull request Jul 31, 2020
* Perpares swift to take optional scalars + adds optional string helper method + disables linters in generated code

* Small fix for generated code

* Update grpc support to alpha 17 for swift
ivannp pushed a commit to ivannp/flatbuffers that referenced this pull request Oct 2, 2020
* Perpares swift to take optional scalars + adds optional string helper method + disables linters in generated code

* Small fix for generated code

* Update grpc support to alpha 17 for swift
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants