Go1 error "err shadowed during return" #11

Open
petar opened this Issue Apr 11, 2012 · 10 comments

2 participants

@petar

I installed the Thrift 0.8.0 package from "brew".

The Go code generated from this package does not compile due to numerous
identical compiler errors:

"err is shadowed during return"

Does this repository here have this problem fixed?
(I don't really know which github version of thrift4go is actually in Thrift 0.8.0)

Thanks
Petar

@pomack
Owner

1) The Go code and Go generator in the official Thrift package came from this repository, but hasn't been updated in the official Thrift package since its original release. Apache has said they prefer release builds and since Go1 is the first real "release" build of Go, I haven't submitted the changes yet. I'll get around to it if enough people push me to ;)
2) The library code in the github repository is tested to work with Go1.
3) The code generator in the github repository has been tested to work with Go1
4) I didn't realize Thrift 0.8.0 was out.
5) I've been testing with Thrift 0.7.something, although others may have been testing with the latest version. So I'm not sure if the generator code will necessarily work with Thrift 0.8.0. You're more than welcome to try. Let me know the results if you do.

@petar

I substituted your t_go_....cc file into the newest Thrift sources and made thrift.

Then I used that thrift to generate some code. It basically works.
You've fixed the bug I had a problem with. I needed to make some minor
changes in the "import" clause of the generated code.

@pomack
Owner

What changes did you have to make to the import clause? Were there extra imports or was the line itself incorrect?

@petar
@pomack
Owner

Issue b should now be resolved. While the spec at http://golang.org/ref/spec#Keywords doesn't mention error as a reserved word, I assume it now is and this just wasn't updated. I've updated the necessary function with a one-liner and it should work now.

@pomack
Owner

Issue a) I don't like the workaround as there could be object names that are hidden due to name conflicts. This type of stuff should also work out of the box. I know where the generated code needs to change, but will need to test this out to ensure I don't break anything.

Can you send me a couple sample files so I can confirm what changes I will make work properly?

@pomack
Owner

Issue c) I wanted to make sure I didn't overwrite some important packages (like os, thrift, etc.) when I installed the library files, so I made all libraries install under the GOROOT/pkg/arch/thriftlib/ directory (back from the r57 days). Starting with Go1, build/install uses the new "go" command that does not require Makefiles but a GOPATH. Non-core library installs (like thrift) should be installed into a local directory along the GOPATH not into the GOROOT/pkg/arch/ directories like they were before. I need to change the directory structure of the generated code, and the Makefiles (since I still like Makefiles, especially if you need to compile from a bunch of directories).

This will take longer to confirm everything is working properly. I also won't push out a build to Apache until issues a) and c) are resolved appropriately.

@petar
@pomack
Owner

Hey Petar,

Matt Proud has been making a lot of changes to thrift4go including a script to merge the source trees. Have you had a chance to try it out?

@pomack
Owner

Hey Petar,

Is this still an issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment