bind: don't use output arg for T in (T, error) returns if T is nullable
The current iOS binding generator only generates returns if the
function being bound does not return an error. If a second error
return type is also present, the binder always generates both the
primary as well as the error as an output parameter.
This is undersirable because most decent functions in Go will
also return errors, so all of those get converted to plain methods
iOS side, each of them requiring allocating the return variable
first and only then execute the call. This gets even more annoying
with the Swift error wrapping protocol which converts errors to
throw statements automatically, but which still needs the ugly pre-
allocs caused by the genrated bindings not returning the result,
just placing it in an output argument.
This CL changes that so that if a nullable result is being returned
by a bound method from Go, then it is generated as a proper return
and not an output argument. This allows erroring functions to still
be called as a function in ObjC, and even more elegantly drop even
the error part in Swift.
Change-Id: I35152d7d2fd2a132eba836fa23be8fd4f317f097
Reviewed-on: https://go-review.googlesource.com/34072
Reviewed-by: Elias Naur <elias.naur@gmail.com>
diff --git a/bind/genobjc.go b/bind/genobjc.go
index c758242..a27302a 100644
--- a/bind/genobjc.go
+++ b/bind/genobjc.go
@@ -490,10 +490,16 @@
if name == "" || paramRE.MatchString(name) {
name = "ret0_"
}
+ typ := res.At(0).Type()
s.retParams = append(s.retParams, paramInfo{
- typ: res.At(0).Type(),
+ typ: typ,
name: name,
})
+ if isNullableType(typ) {
+ s.ret = g.objcType(typ) // Return is nullable, so satisfies the ObjC/Swift error protocol
+ } else {
+ s.ret = "BOOL" // Return is not nullable, must use an output parameter and return bool
+ }
if !isErrorType(res.At(1).Type()) {
g.errorf("second result value must be of type error: %s", obj)
@@ -503,7 +509,6 @@
typ: res.At(1).Type(),
name: "error", // TODO(hyangah): name collision check.
})
- s.ret = "BOOL"
default:
// TODO(hyangah): relax the constraint on multiple return params.
g.errorf("too many result values: %s", obj)
@@ -518,10 +523,12 @@
for _, p := range s.params {
params = append(params, g.objcType(p.typ)+" "+p.name)
}
- if !s.returnsVal() {
- for _, p := range s.retParams {
- params = append(params, g.objcType(p.typ)+"* "+p.name)
- }
+ skip := 0
+ if s.returnsVal() {
+ skip = 1
+ }
+ for _, p := range s.retParams[skip:] {
+ params = append(params, g.objcType(p.typ)+"* "+p.name)
}
return fmt.Sprintf("%s %s%s(%s)", s.ret, g.namePrefix, s.name, strings.Join(params, ", "))
}
@@ -535,14 +542,16 @@
}
params = append(params, fmt.Sprintf("%s:(%s)%s", key, g.objcType(p.typ), p.name))
}
- if !s.returnsVal() {
- for _, p := range s.retParams {
- var key string
- if len(params) > 0 {
- key = p.name
- }
- params = append(params, fmt.Sprintf("%s:(%s)%s", key, g.objcType(p.typ)+"*", p.name))
+ skip := 0
+ if s.returnsVal() {
+ skip = 1
+ }
+ for _, p := range s.retParams[skip:] {
+ var key string
+ if len(params) > 0 {
+ key = p.name
}
+ params = append(params, fmt.Sprintf("%s:(%s)%s", key, g.objcType(p.typ)+"*", p.name))
}
return fmt.Sprintf("(%s)%s%s", s.ret, objcNameReplacer(lowerFirst(s.name)), strings.Join(params, " "))
}
@@ -556,20 +565,22 @@
}
params = append(params, fmt.Sprintf("%s:_%s", key, p.name))
}
- if !s.returnsVal() {
- for _, p := range s.retParams {
- var key string
- if len(params) > 0 {
- key = p.name
- }
- params = append(params, fmt.Sprintf("%s:&%s", key, p.name))
+ skip := 0
+ if s.returnsVal() {
+ skip = 1
+ }
+ for _, p := range s.retParams[skip:] {
+ var key string
+ if len(params) > 0 {
+ key = p.name
}
+ params = append(params, fmt.Sprintf("%s:&%s", key, p.name))
}
return fmt.Sprintf("%s%s", objcNameReplacer(lowerFirst(s.name)), strings.Join(params, " "))
}
func (s *funcSummary) returnsVal() bool {
- return len(s.retParams) == 1 && !isErrorType(s.retParams[0].typ)
+ return (len(s.retParams) == 1 && !isErrorType(s.retParams[0].typ)) || (len(s.retParams) == 2 && isNullableType(s.retParams[0].typ))
}
func (g *ObjcGen) paramName(params *types.Tuple, pos int) string {
@@ -772,27 +783,38 @@
for i, r := range s.retParams {
g.genRead("_"+r.name, fmt.Sprintf("%sr%d", resPrefix, i), r.typ, modeRetained)
}
-
- if !s.returnsVal() {
- for _, p := range s.retParams {
- if isErrorType(p.typ) {
- g.Printf("if (_%s != nil && %s != nil) {\n", p.name, p.name)
- g.Indent()
- g.Printf("*%s = _%s;\n", p.name, p.name)
- g.Outdent()
- g.Printf("}\n")
- } else {
- g.Printf("*%s = _%s;\n", p.name, p.name)
- }
+ skip := 0
+ if s.returnsVal() {
+ skip = 1
+ }
+ for _, p := range s.retParams[skip:] {
+ if isErrorType(p.typ) {
+ g.Printf("if (_%s != nil && %s != nil) {\n", p.name, p.name)
+ g.Indent()
+ g.Printf("*%s = _%s;\n", p.name, p.name)
+ g.Outdent()
+ g.Printf("}\n")
+ } else {
+ g.Printf("*%s = _%s;\n", p.name, p.name)
}
}
if n := len(s.retParams); n > 0 {
- p := s.retParams[n-1]
- if isErrorType(p.typ) {
- g.Printf("return (_%s == nil);\n", p.name)
+ var (
+ first = s.retParams[0]
+ last = s.retParams[n-1]
+ )
+ if (n == 1 && isErrorType(last.typ)) || (n == 2 && !isNullableType(first.typ) && isErrorType(last.typ)) {
+ g.Printf("return (_%s == nil);\n", last.name)
} else {
- g.Printf("return _%s;\n", p.name)
+ if s.returnsVal() && isErrorType(last.typ) {
+ g.Printf("if (_%s != nil) {\n", last.name)
+ g.Indent()
+ g.Printf("return nil;\n")
+ g.Outdent()
+ g.Printf("}\n")
+ }
+ g.Printf("return _%s;\n", first.name)
}
}
}
@@ -896,39 +918,39 @@
}
// call method
- if !s.returnsVal() {
- for _, p := range s.retParams {
- if isErrorType(p.typ) {
- g.Printf("NSError* %s = nil;\n", p.name)
- } else {
- g.Printf("%s %s;\n", g.objcType(p.typ), p.name)
- }
+ for _, p := range s.retParams {
+ if isErrorType(p.typ) {
+ g.Printf("NSError* %s = nil;\n", p.name)
+ } else {
+ g.Printf("%s %s;\n", g.objcType(p.typ), p.name)
}
}
if isErrorType(obj.Type()) && m.Name() == "Error" {
// As a special case, ObjC NSErrors are passed to Go pretending to implement the Go error interface.
// They don't actually have an Error method, so calls to to it needs to be rerouted.
- g.Printf("NSString *returnVal = [o localizedDescription];\n")
+ g.Printf("%s = [o localizedDescription];\n", s.retParams[0].name)
} else {
if s.ret == "void" {
g.Printf("[o %s];\n", s.callMethod(g))
- } else {
+ } else if !s.returnsVal() {
g.Printf("%s returnVal = [o %s];\n", s.ret, s.callMethod(g))
+ } else {
+ g.Printf("%s = [o %s];\n", s.retParams[0].name, s.callMethod(g))
}
}
if len(s.retParams) > 0 {
- if s.returnsVal() { // len(s.retParams) == 1 && s.retParams[0] != error
+ if len(s.retParams) == 1 && !isErrorType(s.retParams[0].typ) {
p := s.retParams[0]
- g.genWrite("returnVal", p.typ, modeRetained)
- g.Printf("return _returnVal;\n")
+ g.genWrite(p.name, p.typ, modeRetained)
+ g.Printf("return _%s;\n", p.name)
} else {
var rets []string
- for i, p := range s.retParams {
+ for _, p := range s.retParams {
if isErrorType(p.typ) {
g.Printf("NSError *_%s = nil;\n", p.name)
- if i == len(s.retParams)-1 { // last param.
+ if !s.returnsVal() {
g.Printf("if (!returnVal) {\n")
} else {
g.Printf("if (%s != nil) {\n", p.name)
diff --git a/bind/objc/SeqTest.m b/bind/objc/SeqTest.m
index 9962c68..d376bc9 100644
--- a/bind/objc/SeqTest.m
+++ b/bind/objc/SeqTest.m
@@ -26,17 +26,13 @@
}
@synthesize value;
-- (BOOL)stringError:(NSString *)s
- ret0_:(NSString **)ret0_
+- (NSString *)stringError:(NSString *)s
error:(NSError **)error {
if ([s isEqualToString:@"number"]) {
- if (ret0_ != NULL) {
- *ret0_ = @"OK";
- }
- return true;
+ return @"OK";
}
*error = [NSError errorWithDomain:@"SeqTest" code:1 userInfo:@{NSLocalizedDescriptionKey: @"NumberError"}];
- return false;
+ return NULL;
}
- (BOOL)error:(BOOL)triggerError error:(NSError **)error {
@@ -301,7 +297,7 @@
GoTestpkgNode *n = GoTestpkgNewNode(@"ErrTest");
n.err = want;
NSError *got = n.err;
- XCTAssertEqual(got, want, @"got different objects efter roundtrip");
+ XCTAssertEqual(got, want, @"got different objects after roundtrip");
NSString *gotMsg = GoTestpkgErrorMessage(want);
XCTAssertEqualObjects(gotMsg, wantMsg, @"err = %@, want %@", gotMsg, wantMsg);
}
@@ -349,14 +345,15 @@
Number *num = [[Number alloc] init];
num.value = 1024;
- NSString *ret;
NSError *error;
- XCTAssertFalse(GoTestpkgCallIStringError(num, @"alphabet", &ret, &error), @"GoTestpkgCallIStringError(Number, 'alphabet') succeeded; want error");
+ NSString *ret = GoTestpkgCallIStringError(num, @"alphabet", &error);
+ XCTAssertNil(ret, @"GoTestpkgCallIStringError(Number, 'alphabet') succeeded(%@); want error", ret);
NSString *desc = [error localizedDescription];
XCTAssertEqualObjects(desc, @"NumberError", @"GoTestpkgCallIStringError(Number, 'alphabet') returned unexpected error message %@", desc);
NSError *error2;
- XCTAssertTrue(GoTestpkgCallIStringError(num, @"number", &ret, &error2), @"GoTestpkgCallIStringError(Number, 'number') failed(%@); want success", error2);
- XCTAssertEqualObjects(ret, @"OK", @"GoTestpkgCallIStringError(Number, 'number') returned unexpected results %@", ret);
+ NSString *ret2 = GoTestpkgCallIStringError(num, @"number", &error2);
+ XCTAssertNotNil(ret2, @"GoTestpkgCallIStringError(Number, 'number') failed(%@); want success", error2);
+ XCTAssertEqualObjects(ret2, @"OK", @"GoTestpkgCallIStringError(Number, 'number') returned unexpected results %@", ret2);
}
- (void)testStrDup:(NSString *)want {
@@ -403,9 +400,8 @@
}
- (void)testReturnsError {
- NSString *value;
NSError *error;
- GoTestpkgReturnsError(TRUE, &value, &error);
+ NSString *value = GoTestpkgReturnsError(TRUE, &error);
NSString *got = [error.userInfo valueForKey:NSLocalizedDescriptionKey];
NSString *want = @"Error";
XCTAssertEqualObjects(got, want, @"want %@\nGoTestpkgReturnsError(TRUE) = (%@, %@)", want, value, got);
diff --git a/bind/testdata/interfaces.objc.m.golden b/bind/testdata/interfaces.objc.m.golden
index 61e23f0..82a859a 100644
--- a/bind/testdata/interfaces.objc.m.golden
+++ b/bind/testdata/interfaces.objc.m.golden
@@ -250,9 +250,10 @@
int32_t cproxyinterfaces_I_Rand(int32_t refnum) {
@autoreleasepool {
GoInterfacesI* o = go_seq_objc_from_refnum(refnum);
- int32_t returnVal = [o rand];
- int32_t _returnVal = (int32_t)returnVal;
- return _returnVal;
+ int32_t ret0_;
+ ret0_ = [o rand];
+ int32_t _ret0_ = (int32_t)ret0_;
+ return _ret0_;
}
}
@@ -273,15 +274,16 @@
int32_t cproxyinterfaces_I3_F(int32_t refnum) {
@autoreleasepool {
GoInterfacesI3* o = go_seq_objc_from_refnum(refnum);
- GoInterfacesI1* returnVal = [o f];
- int32_t _returnVal;
- if ([returnVal conformsToProtocol:@protocol(goSeqRefInterface)]) {
- id<goSeqRefInterface> returnVal_proxy = (id<goSeqRefInterface>)(returnVal);
- _returnVal = go_seq_go_to_refnum(returnVal_proxy._ref);
+ GoInterfacesI1* ret0_;
+ ret0_ = [o f];
+ int32_t _ret0_;
+ if ([ret0_ conformsToProtocol:@protocol(goSeqRefInterface)]) {
+ id<goSeqRefInterface> ret0__proxy = (id<goSeqRefInterface>)(ret0_);
+ _ret0_ = go_seq_go_to_refnum(ret0__proxy._ref);
} else {
- _returnVal = go_seq_to_refnum(returnVal);
+ _ret0_ = go_seq_to_refnum(ret0_);
}
- return _returnVal;
+ return _ret0_;
}
}
@@ -295,18 +297,20 @@
int32_t cproxyinterfaces_LargerI_Rand(int32_t refnum) {
@autoreleasepool {
GoInterfacesLargerI* o = go_seq_objc_from_refnum(refnum);
- int32_t returnVal = [o rand];
- int32_t _returnVal = (int32_t)returnVal;
- return _returnVal;
+ int32_t ret0_;
+ ret0_ = [o rand];
+ int32_t _ret0_ = (int32_t)ret0_;
+ return _ret0_;
}
}
int32_t cproxyinterfaces_SameI_Rand(int32_t refnum) {
@autoreleasepool {
GoInterfacesSameI* o = go_seq_objc_from_refnum(refnum);
- int32_t returnVal = [o rand];
- int32_t _returnVal = (int32_t)returnVal;
- return _returnVal;
+ int32_t ret0_;
+ ret0_ = [o rand];
+ int32_t _ret0_ = (int32_t)ret0_;
+ return _ret0_;
}
}
diff --git a/bind/testdata/issue12403.objc.h.golden b/bind/testdata/issue12403.objc.h.golden
index f77fad2..3248e1e 100644
--- a/bind/testdata/issue12403.objc.h.golden
+++ b/bind/testdata/issue12403.objc.h.golden
@@ -15,7 +15,7 @@
@protocol GoIssue12403Parsable <NSObject>
- (NSString*)fromJSON:(NSString*)jstr;
-- (BOOL)toJSON:(NSString**)ret0_ error:(NSError**)error;
+- (NSString*)toJSON:(NSError**)error;
@end
@class GoIssue12403Parsable;
@@ -26,7 +26,7 @@
- (instancetype)initWithRef:(id)ref;
- (NSString*)fromJSON:(NSString*)jstr;
-- (BOOL)toJSON:(NSString**)ret0_ error:(NSError**)error;
+- (NSString*)toJSON:(NSError**)error;
@end
#endif
diff --git a/bind/testdata/issue12403.objc.m.golden b/bind/testdata/issue12403.objc.m.golden
index a7991ea..ccf7854 100644
--- a/bind/testdata/issue12403.objc.m.golden
+++ b/bind/testdata/issue12403.objc.m.golden
@@ -25,7 +25,7 @@
return _ret0_;
}
-- (BOOL)toJSON:(NSString**)ret0_ error:(NSError**)error {
+- (NSString*)toJSON:(NSError**)error {
int32_t refnum = go_seq_go_to_refnum(self._ref);
struct proxyissue12403_Parsable_ToJSON_return res = proxyissue12403_Parsable_ToJSON(refnum);
NSString *_ret0_ = go_seq_to_objc_string(res.r0);
@@ -37,11 +37,13 @@
_error = [[GoUniverseerror alloc] initWithRef:_error_ref];
}
}
- *ret0_ = _ret0_;
if (_error != nil && error != nil) {
*error = _error;
}
- return (_error == nil);
+ if (_error != nil) {
+ return nil;
+ }
+ return _ret0_;
}
@end
@@ -52,9 +54,10 @@
@autoreleasepool {
GoIssue12403Parsable* o = go_seq_objc_from_refnum(refnum);
NSString *_jstr = go_seq_to_objc_string(jstr);
- NSString* returnVal = [o fromJSON:_jstr];
- nstring _returnVal = go_seq_from_objc_string(returnVal);
- return _returnVal;
+ NSString* ret0_;
+ ret0_ = [o fromJSON:_jstr];
+ nstring _ret0_ = go_seq_from_objc_string(ret0_);
+ return _ret0_;
}
}
@@ -63,10 +66,10 @@
GoIssue12403Parsable* o = go_seq_objc_from_refnum(refnum);
NSString* ret0_;
NSError* error = nil;
- BOOL returnVal = [o toJSON:&ret0_ error:&error];
+ ret0_ = [o toJSON:&error];
nstring _ret0_ = go_seq_from_objc_string(ret0_);
NSError *_error = nil;
- if (!returnVal) {
+ if (error != nil) {
_error = error;
}
int32_t __error;
diff --git a/bind/testdata/structs.objc.h.golden b/bind/testdata/structs.objc.h.golden
index fd09096..6ab4e50 100644
--- a/bind/testdata/structs.objc.h.golden
+++ b/bind/testdata/structs.objc.h.golden
@@ -24,7 +24,7 @@
- (void)setX:(double)v;
- (double)y;
- (void)setY:(double)v;
-- (BOOL)identity:(GoStructsS**)ret0_ error:(NSError**)error;
+- (GoStructsS*)identity:(NSError**)error;
- (double)sum;
@end
@@ -43,7 +43,7 @@
FOUNDATION_EXPORT GoStructsS* GoStructsIdentity(GoStructsS* s);
-FOUNDATION_EXPORT BOOL GoStructsIdentityWithError(GoStructsS* s, GoStructsS** ret0_, NSError** error);
+FOUNDATION_EXPORT GoStructsS* GoStructsIdentityWithError(GoStructsS* s, NSError** error);
@class GoStructsI;
diff --git a/bind/testdata/structs.objc.m.golden b/bind/testdata/structs.objc.m.golden
index 04ba279..d5f648c 100644
--- a/bind/testdata/structs.objc.m.golden
+++ b/bind/testdata/structs.objc.m.golden
@@ -44,7 +44,7 @@
proxystructs_S_Y_Set(refnum, _v);
}
-- (BOOL)identity:(GoStructsS**)ret0_ error:(NSError**)error {
+- (GoStructsS*)identity:(NSError**)error {
int32_t refnum = go_seq_go_to_refnum(self._ref);
struct proxystructs_S_Identity_return res = proxystructs_S_Identity(refnum);
GoStructsS* _ret0_ = nil;
@@ -63,11 +63,13 @@
_error = [[GoUniverseerror alloc] initWithRef:_error_ref];
}
}
- *ret0_ = _ret0_;
if (_error != nil && error != nil) {
*error = _error;
}
- return (_error == nil);
+ if (_error != nil) {
+ return nil;
+ }
+ return _ret0_;
}
- (double)sum {
@@ -143,7 +145,7 @@
return _ret0_;
}
-BOOL GoStructsIdentityWithError(GoStructsS* s, GoStructsS** ret0_, NSError** error) {
+GoStructsS* GoStructsIdentityWithError(GoStructsS* s, NSError** error) {
int32_t _s;
if ([s conformsToProtocol:@protocol(goSeqRefInterface)]) {
id<goSeqRefInterface> s_proxy = (id<goSeqRefInterface>)(s);
@@ -168,11 +170,13 @@
_error = [[GoUniverseerror alloc] initWithRef:_error_ref];
}
}
- *ret0_ = _ret0_;
if (_error != nil && error != nil) {
*error = _error;
}
- return (_error == nil);
+ if (_error != nil) {
+ return nil;
+ }
+ return _ret0_;
}
void cproxystructs_I_M(int32_t refnum) {
diff --git a/bind/types.go b/bind/types.go
index 1b8a6f0..ded3ccf 100644
--- a/bind/types.go
+++ b/bind/types.go
@@ -142,6 +142,10 @@
}
}
+func isNullableType(t types.Type) bool {
+ return types.AssignableTo(types.Typ[types.UntypedNil].Underlying(), t) || t.String() == "string" // string is mapped to NSString*, which is nullable
+}
+
func typePkgFirstElem(t types.Type) string {
nt, ok := t.(*types.Named)
if !ok {
