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

Explicitly convert assignable nil interface values #49

Merged
merged 1 commit into from Oct 5, 2017

Conversation

Projects
None yet
3 participants
@dsnet
Member

dsnet commented Oct 5, 2017

The reflect package has a bug (golang/go#22143).
Let R and T both be interface types and R be assignable to T.
Let F be a function that takes T as its argument.

The Go language lets you directly call F with a nil T or a nil R
(since R is assignable to T) without any special handling.

Currently, reflect allows you to call F with a nil T,
but panics when calling F with a nil R. To avoid this panic,
we explicitly check for the condition where a value:
* is an interface type
* is nil
* is not exactly same as type T
and re-create the value as a nil interface of type T.

@dsnet dsnet requested a review from neild Oct 5, 2017

Explicitly convert assignable nil interface values
The reflect package has a bug (golang/go#22143).
Let R and T both be interface types and R be assignable to T.
Let F be a function that takes T as its argument.

Go language lets you directly call F with a nil T or a nil R
(since R is assignable to T) without any special handling.

Currently, reflect allows you to call F with a nil T,
but panics when F with a nil R. To avoid this panic,
we explicitly check for the condition where a value:
	* is an interface type
	* is nil
	* is not exactly same as type T
and re-create the value as a nil interface of type T.
@neild

neild approved these changes Oct 5, 2017

Wow that's obscure.

@dsnet dsnet merged commit 7ffe192 into master Oct 5, 2017

3 checks passed

cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dsnet dsnet deleted the fix-reflect branch Oct 5, 2017

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Oct 5, 2017

Member

The reflect package has a bug (golang/go#22143).

Fun observation, the GopherJS implementation of reflect package (a custom one) does not have this bug. https://gopherjs.github.io/playground/#/bRzJ0dQ_0W prints 4 trues, as expected.

Member

dmitshur commented Oct 5, 2017

The reflect package has a bug (golang/go#22143).

Fun observation, the GopherJS implementation of reflect package (a custom one) does not have this bug. https://gopherjs.github.io/playground/#/bRzJ0dQ_0W prints 4 trues, as expected.

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