From a4f0a77f2f13243ed95026afcc05380e9953f138 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Wed, 12 Oct 2016 14:05:24 -0400 Subject: [PATCH] bind: param name replacement for invalid unicode names The are three generators that currently call this method: A -> B (1) go -> java (2) go -> objective-c (3) go -> go As discussed below, we only substitute for invalid unicode characters in case (1). **Case 1** Go: From golang.org/ref/spec: Identifiers name program entities such as variables and types. An identifier is a sequence of one or more letters and digits(unicode_digit). The first character in an identifier must be a letter(unicode_letter | "_" ). Java: From https://docs.oracle.com/javase/specs/jls/se7/html/jls-3.html#jls-3.8 `The "Java letters" include uppercase and lowercase ASCII Latin letters A-Z (\u0041-\u005a), and a-z (\u0061-\u007a), and, for historical reasons, the ASCII underscore (_, or \u005f) and dollar sign ($, or \u0024). The $ character should be used only in mechanically generated source code or, rarely, to access pre-existing names on legacy systems.` Therefore, Go's identifiers are checked in case they break these Java rules. **Case 2** There is no objective-c standard specification for valid identifiers. From some testing it seems that Go and objective-c have identical valid identifier rules. **Case 3** Requires no checking. Change-Id: I881810eb9355af6a418727ace32cb6ce4266b2a0 Reviewed-on: https://go-review.googlesource.com/14044 Run-TryBot: Russ Cox Reviewed-by: David Crawshaw --- bind/gen.go | 9 +++-- bind/gengo.go | 16 +++++---- bind/genjava.go | 43 ++++++++++++++--------- bind/genobjc.go | 10 ++++-- bind/testdata/issue10788.go | 2 +- bind/testdata/issue10788.go.golden | 12 +++---- bind/testdata/issue10788.objc.go.h.golden | 2 +- bind/testdata/issue10788.objc.h.golden | 4 +-- bind/testdata/issue10788.objc.m.golden | 12 +++---- 9 files changed, 64 insertions(+), 46 deletions(-) diff --git a/bind/gen.go b/bind/gen.go index 7d4ad65..1878142 100644 --- a/bind/gen.go +++ b/bind/gen.go @@ -266,7 +266,7 @@ func (g *Generator) cgoType(t types.Type) string { return "TODO" } -func (g *Generator) genInterfaceMethodSignature(m *types.Func, iName string, header bool) { +func (g *Generator) genInterfaceMethodSignature(m *types.Func, iName string, header bool, g_paramName func(*types.Tuple, int) string) { sig := m.Type().(*types.Signature) params := sig.Params() res := sig.Results() @@ -293,7 +293,7 @@ func (g *Generator) genInterfaceMethodSignature(m *types.Func, iName string, hea g.Printf("cproxy%s_%s_%s(int32_t refnum", g.pkgPrefix, iName, m.Name()) for i := 0; i < params.Len(); i++ { t := params.At(i).Type() - g.Printf(", %s %s", g.cgoType(t), paramName(params, i)) + g.Printf(", %s %s", g.cgoType(t), g_paramName(params, i)) } g.Printf(")") if header { @@ -360,10 +360,9 @@ func (g *Generator) isSupported(t types.Type) bool { var paramRE = regexp.MustCompile(`^p[0-9]*$`) -// paramName replaces incompatible name with a p0-pN name. +// basicParamName replaces incompatible name with a p0-pN name. // Missing names, or existing names of the form p[0-9] are incompatible. -// TODO(crawshaw): Replace invalid unicode names. -func paramName(params *types.Tuple, pos int) string { +func basicParamName(params *types.Tuple, pos int) string { name := params.At(pos).Name() if name == "" || name[0] == '_' || paramRE.MatchString(name) { name = fmt.Sprintf("p%d", pos) diff --git a/bind/gengo.go b/bind/gengo.go index ec26ffc..9bbba4d 100644 --- a/bind/gengo.go +++ b/bind/gengo.go @@ -40,7 +40,7 @@ func (g *goGen) genFuncBody(o *types.Func, selectorLHS string) { params := sig.Params() for i := 0; i < params.Len(); i++ { p := params.At(i) - pn := "param_" + paramName(params, i) + pn := "param_" + g.paramName(params, i) g.genRead("_"+pn, pn, p.Type(), modeTransient) } @@ -64,7 +64,7 @@ func (g *goGen) genFuncBody(o *types.Func, selectorLHS string) { if i > 0 { g.Printf(", ") } - g.Printf("_param_%s", paramName(params, i)) + g.Printf("_param_%s", g.paramName(params, i)) } g.Printf(")\n") @@ -152,7 +152,7 @@ func (g *goGen) genFuncSignature(o *types.Func, objName string) { g.Printf(", ") } p := params.At(i) - g.Printf("param_%s C.%s", paramName(params, i), g.cgoType(p.Type())) + g.Printf("param_%s C.%s", g.paramName(params, i), g.cgoType(p.Type())) } g.Printf(") ") res := sig.Results() @@ -169,6 +169,10 @@ func (g *goGen) genFuncSignature(o *types.Func, objName string) { g.Printf("{\n") } +func (g *goGen) paramName(params *types.Tuple, pos int) string { + return basicParamName(params, pos) +} + func (g *goGen) genFunc(o *types.Func) { if !g.isSigSupported(o.Type()) { g.Printf("// skipped function %s with unsupported parameter or result types\n", o.Name()) @@ -314,7 +318,7 @@ func (g *goGen) genInterface(obj *types.TypeName) { if i > 0 { g.Printf(", ") } - g.Printf("param_%s %s", paramName(params, i), g.typeString(params.At(i).Type())) + g.Printf("param_%s %s", g.paramName(params, i), g.typeString(params.At(i).Type())) } g.Printf(") ") @@ -327,7 +331,7 @@ func (g *goGen) genInterface(obj *types.TypeName) { g.Indent() for i := 0; i < params.Len(); i++ { - pn := "param_" + paramName(params, i) + pn := "param_" + g.paramName(params, i) g.genWrite("_"+pn, pn, params.At(i).Type(), modeTransient) } @@ -336,7 +340,7 @@ func (g *goGen) genInterface(obj *types.TypeName) { } g.Printf("C.cproxy%s_%s_%s(C.int32_t(p.Bind_proxy_refnum__())", g.pkgPrefix, obj.Name(), m.Name()) for i := 0; i < params.Len(); i++ { - g.Printf(", _param_%s", paramName(params, i)) + g.Printf(", _param_%s", g.paramName(params, i)) } g.Printf(")\n") var retName string diff --git a/bind/genjava.go b/bind/genjava.go index 3fb71e0..99ab7aa 100644 --- a/bind/genjava.go +++ b/bind/genjava.go @@ -9,6 +9,7 @@ import ( "go/constant" "go/types" "math" + "regexp" "strings" "golang.org/x/mobile/internal/importers/java" @@ -274,7 +275,7 @@ func (g *JavaGen) genConstructor(f *types.Func, n string, jcls bool) { if i > 0 { g.Printf(", ") } - g.Printf(paramName(params, i)) + g.Printf(g.paramName(params, i)) } g.Printf(");\n") } @@ -284,7 +285,7 @@ func (g *JavaGen) genConstructor(f *types.Func, n string, jcls bool) { if i > 0 { g.Printf(", ") } - g.Printf(paramName(params, i)) + g.Printf(g.paramName(params, i)) } g.Printf(");\n") g.Outdent() @@ -310,7 +311,7 @@ func (g *JavaGen) genFuncArgs(f *types.Func, jm *java.Func, hasThis bool) { g.Printf(", ") } v := params.At(i) - name := paramName(params, i) + name := g.paramName(params, i) jt := g.javaType(v.Type()) g.Printf("%s %s", jt, name) } @@ -615,7 +616,7 @@ func (g *JavaGen) genJNIFuncSignature(o *types.Func, sName string, jm *java.Func for ; i < params.Len(); i++ { g.Printf(", ") v := sig.Params().At(i) - name := paramName(params, i) + name := g.paramName(params, i) jt := g.jniType(v.Type()) g.Printf("%s %s", jt, name) } @@ -626,6 +627,16 @@ func (g *JavaGen) jniPkgName() string { return strings.Replace(g.javaPkgName(g.Pkg), ".", "_", -1) } +var javaLetterDigitRE = regexp.MustCompile(`[0-9a-zA-Z$_]`) + +func (g *JavaGen) paramName(params *types.Tuple, pos int) string { + name := basicParamName(params, pos) + if !javaLetterDigitRE.MatchString(name) { + name = fmt.Sprintf("p%d", pos) + } + return name +} + func (g *JavaGen) genFuncSignature(o *types.Func, jm *java.Func, hasThis bool) { sig := o.Type().(*types.Signature) res := sig.Results() @@ -928,12 +939,12 @@ func (g *JavaGen) genJNIConstructor(f *types.Func, sName string) { for i := 0; i < params.Len(); i++ { v := params.At(i) jt := g.jniType(v.Type()) - g.Printf(", %s %s", jt, paramName(params, i)) + g.Printf(", %s %s", jt, g.paramName(params, i)) } g.Printf(") {\n") g.Indent() for i := 0; i < params.Len(); i++ { - name := paramName(params, i) + name := g.paramName(params, i) g.genJavaToC(name, params.At(i).Type(), modeTransient) } // Constructors always have one result parameter, a *T. @@ -942,11 +953,11 @@ func (g *JavaGen) genJNIConstructor(f *types.Func, sName string) { if i > 0 { g.Printf(", ") } - g.Printf("_%s", paramName(params, i)) + g.Printf("_%s", g.paramName(params, i)) } g.Printf(");\n") for i := 0; i < params.Len(); i++ { - g.genRelease(paramName(params, i), params.At(i).Type(), modeTransient) + g.genRelease(g.paramName(params, i), params.At(i).Type(), modeTransient) } // Pass no proxy class so that the Seq.Ref is returned instead. g.Printf("return go_seq_from_refnum(env, refnum, NULL, NULL);\n") @@ -989,10 +1000,10 @@ func (g *JavaGen) genJNIFuncBody(o *types.Func, sName string, jm *java.Func, isj if isjava && params.Len() > 0 && params.At(0).Name() == "this" { // Start after the implicit this argument. first = 1 - g.Printf("int32_t _%s = go_seq_to_refnum(env, __this__);\n", paramName(params, 0)) + g.Printf("int32_t _%s = go_seq_to_refnum(env, __this__);\n", g.paramName(params, 0)) } for i := first; i < params.Len(); i++ { - name := paramName(params, i) + name := g.paramName(params, i) g.genJavaToC(name, params.At(i).Type(), modeTransient) } resPrefix := "" @@ -1013,11 +1024,11 @@ func (g *JavaGen) genJNIFuncBody(o *types.Func, sName string, jm *java.Func, isj if i > 0 || sName != "" { g.Printf(", ") } - g.Printf("_%s", paramName(params, i)) + g.Printf("_%s", g.paramName(params, i)) } g.Printf(");\n") for i := first; i < params.Len(); i++ { - g.genRelease(paramName(params, i), params.At(i).Type(), modeTransient) + g.genRelease(g.paramName(params, i), params.At(i).Type(), modeTransient) } for i := 0; i < res.Len(); i++ { tn := fmt.Sprintf("_r%d", i) @@ -1061,12 +1072,12 @@ func (g *JavaGen) genMethodInterfaceProxy(oName string, m *types.Func) { sig := m.Type().(*types.Signature) params := sig.Params() res := sig.Results() - g.genInterfaceMethodSignature(m, oName, false) + g.genInterfaceMethodSignature(m, oName, false, g.paramName) g.Indent() g.Printf("JNIEnv *env = go_seq_push_local_frame(%d);\n", params.Len()) g.Printf("jobject o = go_seq_from_refnum(env, refnum, proxy_class_%s_%s, proxy_class_%s_%s_cons);\n", g.pkgPrefix, oName, g.pkgPrefix, oName) for i := 0; i < params.Len(); i++ { - pn := paramName(params, i) + pn := g.paramName(params, i) g.genCToJava("_"+pn, pn, params.At(i).Type(), modeTransient) } if res.Len() > 0 && !isErrorType(res.At(0).Type()) { @@ -1077,7 +1088,7 @@ func (g *JavaGen) genMethodInterfaceProxy(oName string, m *types.Func) { } g.Printf("mid_%s_%s", oName, m.Name()) for i := 0; i < params.Len(); i++ { - g.Printf(", _%s", paramName(params, i)) + g.Printf(", _%s", g.paramName(params, i)) } g.Printf(");\n") var retName string @@ -1127,7 +1138,7 @@ func (g *JavaGen) GenH() error { g.Printf("// skipped method %s.%s with unsupported parameter or return types\n\n", iface.obj.Name(), m.Name()) continue } - g.genInterfaceMethodSignature(m, iface.obj.Name(), true) + g.genInterfaceMethodSignature(m, iface.obj.Name(), true, g.paramName) g.Printf("\n") } } diff --git a/bind/genobjc.go b/bind/genobjc.go index b1ab0ff..9d3f9e9 100644 --- a/bind/genobjc.go +++ b/bind/genobjc.go @@ -64,7 +64,7 @@ func (g *objcGen) genGoH() error { g.Printf("// skipped method %s.%s with unsupported parameter or return types\n\n", i.obj.Name(), m.Name()) continue } - g.genInterfaceMethodSignature(m, i.obj.Name(), true) + g.genInterfaceMethodSignature(m, i.obj.Name(), true, g.paramName) g.Printf("\n") } } @@ -366,7 +366,7 @@ func (g *objcGen) funcSummary(obj *types.Func) *funcSummary { p := params.At(i) v := paramInfo{ typ: p.Type(), - name: paramName(params, i), + name: g.paramName(params, i), } s.params = append(s.params, v) } @@ -479,6 +479,10 @@ func (s *funcSummary) returnsVal() bool { return len(s.retParams) == 1 && !isErrorType(s.retParams[0].typ) } +func (g *objcGen) paramName(params *types.Tuple, pos int) string { + return basicParamName(params, pos) +} + func (g *objcGen) genFuncH(obj *types.Func) { if !g.isSigSupported(obj.Type()) { g.Printf("// skipped function %s with unsupported parameter or return types\n\n", obj.Name()) @@ -776,7 +780,7 @@ func (g *objcGen) genInterfaceM(obj *types.TypeName, t *types.Interface) bool { func (g *objcGen) genInterfaceMethodProxy(obj *types.TypeName, m *types.Func) { oName := obj.Name() s := g.funcSummary(m) - g.genInterfaceMethodSignature(m, oName, false) + g.genInterfaceMethodSignature(m, oName, false, g.paramName) g.Indent() g.Printf("@autoreleasepool {\n") g.Indent() diff --git a/bind/testdata/issue10788.go b/bind/testdata/issue10788.go index e5c6a12..af48721 100644 --- a/bind/testdata/issue10788.go +++ b/bind/testdata/issue10788.go @@ -10,5 +10,5 @@ type TestStruct struct { type TestInterface interface { DoSomeWork(s *TestStruct) - MultipleUnnamedParams(_ int, p0 string, _ int64) + MultipleUnnamedParams(_ int, p0 string, 日本 int64) } diff --git a/bind/testdata/issue10788.go.golden b/bind/testdata/issue10788.go.golden index 1d3652a..a6ef182 100644 --- a/bind/testdata/issue10788.go.golden +++ b/bind/testdata/issue10788.go.golden @@ -52,13 +52,13 @@ func proxyissue10788_TestInterface_DoSomeWork(refnum C.int32_t, param_s C.int32_ } //export proxyissue10788_TestInterface_MultipleUnnamedParams -func proxyissue10788_TestInterface_MultipleUnnamedParams(refnum C.int32_t, param_p0 C.nint, param_p1 C.nstring, param_p2 C.int64_t) { +func proxyissue10788_TestInterface_MultipleUnnamedParams(refnum C.int32_t, param_p0 C.nint, param_p1 C.nstring, param_日本 C.int64_t) { ref := _seq.FromRefNum(int32(refnum)) v := ref.Get().(issue10788.TestInterface) _param_p0 := int(param_p0) _param_p1 := decodeString(param_p1) - _param_p2 := int64(param_p2) - v.MultipleUnnamedParams(_param_p0, _param_p1, _param_p2) + _param_日本 := int64(param_日本) + v.MultipleUnnamedParams(_param_p0, _param_p1, _param_日本) } type proxyissue10788_TestInterface _seq.Ref @@ -75,9 +75,9 @@ func (p *proxyissue10788_TestInterface) DoSomeWork(param_s *issue10788.TestStruc C.cproxyissue10788_TestInterface_DoSomeWork(C.int32_t(p.Bind_proxy_refnum__()), _param_s) } -func (p *proxyissue10788_TestInterface) MultipleUnnamedParams(param_p0 int, param_p1 string, param_p2 int64) { +func (p *proxyissue10788_TestInterface) MultipleUnnamedParams(param_p0 int, param_p1 string, param_日本 int64) { _param_p0 := C.nint(param_p0) _param_p1 := encodeString(param_p1) - _param_p2 := C.int64_t(param_p2) - C.cproxyissue10788_TestInterface_MultipleUnnamedParams(C.int32_t(p.Bind_proxy_refnum__()), _param_p0, _param_p1, _param_p2) + _param_日本 := C.int64_t(param_日本) + C.cproxyissue10788_TestInterface_MultipleUnnamedParams(C.int32_t(p.Bind_proxy_refnum__()), _param_p0, _param_p1, _param_日本) } diff --git a/bind/testdata/issue10788.objc.go.h.golden b/bind/testdata/issue10788.objc.go.h.golden index 5c480a8..28f3c46 100644 --- a/bind/testdata/issue10788.objc.go.h.golden +++ b/bind/testdata/issue10788.objc.go.h.golden @@ -10,6 +10,6 @@ #include void cproxyissue10788_TestInterface_DoSomeWork(int32_t refnum, int32_t s); -void cproxyissue10788_TestInterface_MultipleUnnamedParams(int32_t refnum, nint p0, nstring p1, int64_t p2); +void cproxyissue10788_TestInterface_MultipleUnnamedParams(int32_t refnum, nint p0, nstring p1, int64_t 日本); #endif diff --git a/bind/testdata/issue10788.objc.h.golden b/bind/testdata/issue10788.objc.h.golden index 25c2e7d..5bb4a39 100644 --- a/bind/testdata/issue10788.objc.h.golden +++ b/bind/testdata/issue10788.objc.h.golden @@ -24,7 +24,7 @@ @protocol GoIssue10788TestInterface - (void)doSomeWork:(GoIssue10788TestStruct*)s; -- (void)multipleUnnamedParams:(long)p0 p1:(NSString*)p1 p2:(int64_t)p2; +- (void)multipleUnnamedParams:(long)p0 p1:(NSString*)p1 日本:(int64_t)日本; @end @class GoIssue10788TestInterface; @@ -35,7 +35,7 @@ - (instancetype)initWithRef:(id)ref; - (void)doSomeWork:(GoIssue10788TestStruct*)s; -- (void)multipleUnnamedParams:(long)p0 p1:(NSString*)p1 p2:(int64_t)p2; +- (void)multipleUnnamedParams:(long)p0 p1:(NSString*)p1 日本:(int64_t)日本; @end #endif diff --git a/bind/testdata/issue10788.objc.m.golden b/bind/testdata/issue10788.objc.m.golden index 80fc8a5..5d99e1f 100644 --- a/bind/testdata/issue10788.objc.m.golden +++ b/bind/testdata/issue10788.objc.m.golden @@ -54,12 +54,12 @@ proxyissue10788_TestInterface_DoSomeWork(refnum, _s); } -- (void)multipleUnnamedParams:(long)p0 p1:(NSString*)p1 p2:(int64_t)p2 { +- (void)multipleUnnamedParams:(long)p0 p1:(NSString*)p1 日本:(int64_t)日本 { int32_t refnum = go_seq_go_to_refnum(self._ref); nint _p0 = (nint)p0; nstring _p1 = go_seq_from_objc_string(p1); - int64_t _p2 = (int64_t)p2; - proxyissue10788_TestInterface_MultipleUnnamedParams(refnum, _p0, _p1, _p2); + int64_t _日本 = (int64_t)日本; + proxyissue10788_TestInterface_MultipleUnnamedParams(refnum, _p0, _p1, _日本); } @end @@ -81,13 +81,13 @@ void cproxyissue10788_TestInterface_DoSomeWork(int32_t refnum, int32_t s) { } } -void cproxyissue10788_TestInterface_MultipleUnnamedParams(int32_t refnum, nint p0, nstring p1, int64_t p2) { +void cproxyissue10788_TestInterface_MultipleUnnamedParams(int32_t refnum, nint p0, nstring p1, int64_t 日本) { @autoreleasepool { GoIssue10788TestInterface* o = go_seq_objc_from_refnum(refnum); long _p0 = (long)p0; NSString *_p1 = go_seq_to_objc_string(p1); - int64_t _p2 = (int64_t)p2; - [o multipleUnnamedParams:_p0 p1:_p1 p2:_p2]; + int64_t _日本 = (int64_t)日本; + [o multipleUnnamedParams:_p0 p1:_p1 日本:_日本]; } }