Skip to content

Support Go 1.16 reflect package. #996

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

Merged
merged 9 commits into from
Mar 16, 2021

Conversation

nevkontakte
Copy link
Member

This pull request addresses multiple issues that were causing test failures for the reflect package.

  • Better support for determining caller function name and files.
  • Stricter type checks in the MakeFunc() function.
  • Working implementation of the StructOf() function.
  • Fixed minor a divergence with the upstream Go in type signature string generation.
  • A few minor internal changes to match changes in the upstream.

See individual commit messages for more details.

Updates #992.

@nevkontakte nevkontakte requested a review from flimzy March 13, 2021 17:52
@nevkontakte nevkontakte changed the base branch from master to go1.16-stdlib March 13, 2021 17:52
Copy link
Member

@flimzy flimzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't worry about comment typo corrections unless you're really bothered, or are making other changes anyway. It's still readable :)

nevkontakte and others added 6 commits March 16, 2021 10:35
`FuncForPC()` returns function/file/line metadata for any given code
position (effectively a memory address). However, since GopherJS runtime
doesn't have access to raw addresses, we emulate this by assigning
virtual position counters for call places seen by Caller() and use them
to retrieve the original function information.

This is a more limited implementation that the upstream, but I expect it
to work for most of the real-life cases, and it fixes a few test
failures in the `reflect` package. With this it should be fairly easy to
bring back proper support for `testing.T.Helper()` function.

Co-authored-by: Jonathan Hall <flimzy@flimzy.com>
Previously we could return anything that may or may not have matched the
required function signature, causing hard to debug failures in the code.
This change replicates the upstream Go behavior.
In the upstream it is implemented in `runtime` and exported into
`reflect` via a go:linkname directive, but GopherJS doesn't support that
and we already define a local typeOffList in `reflect` as a substitute.

Also delete it from reflectlite, since it isn't actually used there.

Co-authored-by: visualfc <visualfc@gmail.com>
It has been introduced all the way back in Go 1.10, but GopherJS never
properly supported it. The new implementation seems to pass most of the
tests, which is an improvement. There are a couple of tests that are
still failing, but fixing that seems like a harder task.

The code is based on goplusjs/gopherjs@a293d52.

Co-authored-by: visualfc <visualfc@gmail.com>
This reflects a change in the upstream intended to make use of terms
"value" and "element" more consistent in the context of maps.
nevkontakte and others added 2 commits March 16, 2021 10:58
The original test was using a signalling NaN, but JavaScript appears to
coerce all NaNs to quiet ones. I found no mention of this behavior in
the spec, but I suspect it is just the behavior JavaScript historically
implemented when dealing with NaNs.

Co-authored-by: Jonathan Hall <flimzy@flimzy.com>
Several `reflect` tests rely on it to execute correctly.
@nevkontakte nevkontakte merged commit f5092bf into gopherjs:go1.16-stdlib Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants