Thanks for reporting Andrew. I can reproduce.

I've looked into it a little and I have a fairly good suspicion of what's causing this. At first, I thought it might have to do with the 😱 import graph of grpc that I saw in wire.pb.go. However, that's not it.

If you look at the trace, it all happens within github.com/gogo/protobuf/proto package. It starts out in proto.Marshal, that calls (*proto.Buffer) Marshal method, which calls enc_struct and that finally calls enc_ref_string where the problem happens. Its code:

// Encode a reference to a string pointer.
func (o *Buffer) enc_ref_string(p *Properties, base structPointer) error {
    v := *structPointer_StringVal(base, p.field)
    o.buf = append(o.buf, p.tagcode...)
    o.EncodeStringBytes(v)
    return nil
}

The JS error hints at the problem:

Uncaught TypeError: structPointer_StringVal(...).$get is not a function

It's on the v := *structPointer_StringVal(base, p.field) line. If you look at structPointer_StringVal, that func uses unsafe package.

// StringVal returns the address of a string field in the struct.
func structPointer_StringVal(p structPointer, f field) *string {
    return (*string)(unsafe.Pointer(uintptr(p) + uintptr(f)))
}

GopherJS does not support that because it doesn't support unsafe package, see its compatibility table.

However, there is hope. I see that there are two implementations of that code, one that uses unsafe, and the other uses reflect (it's created for appengine where unsafe is not supported, just like with GopherJS). So perhaps by using that, you can get proto3 to work.

I tried it by doing gopherjs build --tags=appengine, but it looks like gogo doesn't actually have a complete implementation in pointer_reflect.go compared to pointer_unsafe_gogo.go. I got errors like:

$ gopherjs build --tags=appengine
github.com/gogo/protobuf/proto/decode_gogo.go:50:9: undeclared name: structPointer_FieldPointer
github.com/gogo/protobuf/proto/decode_gogo.go:61:12: undeclared name: appendStructPointer
github.com/gogo/protobuf/proto/decode_gogo.go:170:12: undeclared name: appendStructPointer

Indeed, structPointer_FieldPointer is not present in pointer_reflect.go, but it is in pointer_unsafe_gogo.go.

However, if you use plain protobuf rather than gogoproto, then it works.

1. Change your import from github.com/gogo/protobuf/proto to github.com/golang/protobuf/proto in fail.go. See, e.g., here.
2. Build with gopherjs build --tags=appengine. (You may need to clear $GOPATH/pkg/*_js cache when building with GopherJS and using different build tags, it doesn't seem to handle that situation well, but that's a bug that'll be resolved once proposal: Replace in-house implementation of gopherjs build tool with augmented forked cmd/go build tool. #388 is done.)

Those packages could change // +build appengine to be // +build appengine js or similar if they wanted to support JS architecture without you having to supply appengine build tag yourself.

Ugh, I didn't notice that gogo had somehow snuck into my GOPATH workspace.

I intended to use the canonical version of the package at "github.com/golang/protobuf/proto", which works with the appropriate build tag. I will arrange to have the js tag added to the canonical proto package.

I have changed the title of this issue. I think I would figured this out myself if gopherjs had said "You can't use unsafe." I think this can be detected at compile time.

Is it possible to make the diagnostics better?

"You can't use unsafe." I think this can be detected at compile time.

Is it possible to make the diagnostics better?

You're not the first person that ran into this. I think we should do that. Let's use this issue to track it.

Ugh, I didn't notice that gogo had somehow snuck into my GOPATH workspace.

goimports, maybe? Edit: Ah, never mind, you said "into GOPATH workspace", not "into my .go file". In that case, it was probably a go get that's responsible. :P

On 22 March 2016 at 15:50, Dmitri Shuralyov notifications@github.com
wrote:

goimports, maybe?

Yes, through goimports.

In order to resolve this issue, we want to detect when unsafe is imported (even if indirectly). Suppose that's done, what would be ideal next step?

Should GopherJS print a "warning" saying "unsafe" is imported but it's not supported?

But warnings are not in the spirit of Go compilers.

Perhaps it should result in a fatal "unsafe is imported, but it's not supported" error instead. I am in favor of doing this. Any objections?

Would it be possible to override that behavior some how, (command line flag?) to permit the knowing compilation of such code, if the "unsafe" behavior is non-essential?

OTOH, although Go compilers may not traditionally warn, GopherJS does in a few other cases, most notably for syscalls. Is this case fundamentally different?

I would likely object to adding a new flag; let's try to find a solution for this that doesn't require adding flags.

I see your point, we could treat "unsafe" similarly to "os" and syscall packages and print a warning, as GopherJS already does in those cases.

But is that ideal? The Go compiler could also have resorted to printing warnings for some things, but they were very vigilant about not letting that become the norm, and I think we're all thankful that Go compiler has no warnings. Should not GopherJS aim to same high standard? I don't want to see it degrade to the sad state like that in the world of C++, for example, see here.

Perhaps that's an issue to be solved as a followup, and for now we could treat unsafe in similar ways to syscalls and emit warnings...

At the same time, I'm not sure how it makes sense to compile code that imports unsafe when the package is not supported. That's like asking the compiler to do something it says it can't. What's the value?

I do prefer the Go philosophy of aborting a compile rather than warning. However, as GopherJS is a port, it doesn't have the same power to make the "best" design decisions, which may put it in a bit of a bind in this area. If GopherJS were the default compiler, it would be quite reasonable to simply abort the compile when an un-supported thing was being done. As the goal here, rather, is to be "as compatible as reasonably possible" with another spec, that may not be the best approach.

It seems to me that there aren't a lot of ways to handle these situations.

1. Abort the compile.
2. Panic at run-time when an unsupported method is called.
3. Return a place-holder value.
4. Add support for the otherwise unsupported methods.

On top of these basic behaviors, we also have the option to warn (or not) in cases 2 (at compile time), 3 & 4 (at runtime).

#2 seems pretty evil to me. #4 isn't always possible.

#3 seems to be the approach taken for 'os' and syscalls.

Unless I'm overlooking some other approach, #3 seems like it's probably the best option for 'unsafe', too.

While I agree with you that a new flag ought to be a last resort, if warning + dummy values isn't always wanted, perhaps a "strict" mode could be added (perhaps as the default). In this mode, GopherJS would abort the compilation any time an unsupported library was imported. I envision it affecting all of these cases, not just 'unsafe'.

FWIW, GopherJS supports some uses of unsafe, albeit inconsistently and not very safely. I think this is a good candidate for something like ‘gopherjs lint’, since linters seem to be an idiomatic way of detecting possible errors. What I am concerned about is the number of false positives this may bring.

One other option is to clearly define unsafe semantics for GopherJS and fail compilation when unsupported operations are detected, such as pointer arithmetic. I think there is a subset of most common unsafe operations GopherJS could simulate, for example pointer arithmetics within a single byte array, but it requires some work.