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

Go 1.11.1 support (version bump to GopherJS 1.11-2). #865

Merged
merged 2 commits into from Oct 4, 2018

Conversation

Projects
None yet
2 participants
@dmitshur
Member

dmitshur commented Oct 3, 2018

Go 1.11.1 has been released recently. It includes some changes to the
reflect package, including:

The second commit makes some changes to internal reflect code that
makes GopherJS reflect overrides no longer compatible, causing the
package to no longer build:

$ gopherjs build reflect
/usr/local/go/src/reflect/value.go:673:60: cannot use t (variable of type *rtype) as *funcType value in argument to funcLayout
/usr/local/go/src/reflect/value.go:336:21: cannot use methodReceiver(op, v, int(v.flag) >> flagMethodShift) (value of type *rtype) as *funcType value in assignment

Specifically, the type of return value t of function methodReceiver
was changed from *rtype to *funcType in commit golang/go@3afa9df.

This change updates the native reflect override for methodReceiver
function and Value.call method accordingly.

Regenerate natives with:

go generate ./compiler/natives

Use Go 1.11.1 in CI.

Bump GopherJS version to GopherJS 1.11-2.

Fixes #862. /cc @bradfitz @katiehockman

Go 1.11.1 support (version bump to GopherJS 1.11-2).
Go 1.11.1 has been released recently. It includes some changes to the
reflect package, including:

-	golang/go@a2f1c8e
-	golang/go@3afa9df
-	golang/go@58c9bd9

The second commit makes some changes to internal reflect code that
makes GopherJS reflect overrides no longer compatible, causing the
package to no longer build:

	$ gopherjs build reflect
	/usr/local/go/src/reflect/value.go:673:60: cannot use t (variable of type *rtype) as *funcType value in argument to funcLayout
	/usr/local/go/src/reflect/value.go:336:21: cannot use methodReceiver(op, v, int(v.flag) >> flagMethodShift) (value of type *rtype) as *funcType value in assignment

Specifically, the type of return value t of function methodReceiver
was changed from *rtype to *funcType in commit golang/go@3afa9df.

This change updates the native reflect override for methodReceiver
function and Value.call method accordingly.

Regenerate natives with:

	go generate ./compiler/natives

Use Go 1.11.1 in CI.

Bump GopherJS version to GopherJS 1.11-2.

Fixes #862.
@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Oct 3, 2018

LGTM. It's not a goal for GopherJS to still also support Go 1.11(.0) is it? (Doesn't seem worthwhile if it's difficult.)

bradfitz commented Oct 3, 2018

LGTM. It's not a goal for GopherJS to still also support Go 1.11(.0) is it? (Doesn't seem worthwhile if it's difficult.)

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Oct 3, 2018

Member

It's not a goal for GopherJS to still also support Go 1.11(.0) is it? (Doesn't seem worthwhile if it's difficult.)

It would be a goal, but I didn't see any easy way of making it happen (there are only build tags for go1.10, go1.11, etc., not minor point releases), and figured it's fine to require latest patch version of Go 1.11.

But now that I think about it, it wouldn't be too hard to special-case it in github.com/gopherjs/gopherjs/build package. I'll try that this time.

Member

dmitshur commented Oct 3, 2018

It's not a goal for GopherJS to still also support Go 1.11(.0) is it? (Doesn't seem worthwhile if it's difficult.)

It would be a goal, but I didn't see any easy way of making it happen (there are only build tags for go1.10, go1.11, etc., not minor point releases), and figured it's fine to require latest patch version of Go 1.11.

But now that I think about it, it wouldn't be too hard to special-case it in github.com/gopherjs/gopherjs/build package. I'll try that this time.

Restore backwards compatibility with Go 1.11(.0).
Create a custom go1.11.1 release tag (to be applied whenever Go version
is 1.11.1 or greater) and use it to select the appropriate overrides
for the reflect package for Go 1.11.1 and Go 1.11(.0).

This way, GopherJS 1.11-2 will function with Go 1.11.1 and Go 1.11(.0)
versions, rather than only with Go 1.11.1.

Regenerate ./compiler/natives.
@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Oct 3, 2018

Member

I'm not completely happy with the complexity this adds, but the benefit of having GopherJS 1.11-2 be able to build and install regardless if the user has Go 1.11.1 or Go 1.11(.0) seems worthwhile. PTAL.

Edit: CI is passing both on Go 1.11.1 and 1.11(.0).

Member

dmitshur commented Oct 3, 2018

I'm not completely happy with the complexity this adds, but the benefit of having GopherJS 1.11-2 be able to build and install regardless if the user has Go 1.11.1 or Go 1.11(.0) seems worthwhile. PTAL.

Edit: CI is passing both on Go 1.11.1 and 1.11(.0).

@damz damz referenced this pull request Oct 3, 2018

Open

Add support for WebAssembly #215

@dmitshur dmitshur merged commit 0fee806 into master Oct 4, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

@dmitshur dmitshur deleted the go1.11.1-reflect branch Oct 4, 2018

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