Skip to content
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

interp: do not wrap empty interface #1017

Merged
merged 13 commits into from
Feb 4, 2021
Merged

interp: do not wrap empty interface #1017

merged 13 commits into from
Feb 4, 2021

Conversation

mpl
Copy link
Collaborator

@mpl mpl commented Jan 25, 2021

The empty interface (interface{}), and its variants (such as []interface{} and map[string]interface{}), are commonly used in Go to (json) Unmarshal arbitrary data. Within Yaegi, all interface types are wrapped in a valueInterface struct in order to retain all the information needed for a consistent internal state (as reflect is not enough to achieve that). However, this wrapping ends up being problematic when it comes to the type assertions related to the aforementioned Unmarshaling.

Therefore, this PR is an attempt to consider the empty interface (and its variants) as an exception within Yaegi, that should never be wrapped within a valueInterface, and to treat it similarly to the other basic Go types. The assumption is that the wrapping should not be needed, as there is no information about implemented methods to maintain.

Fixes #984
Fixes #829
Fixes #1015

internal/cmd/genop/genop.go Show resolved Hide resolved
internal/cmd/genop/genop.go Show resolved Hide resolved
@mvertes mvertes added area/core bug Something isn't working labels Feb 2, 2021
@mvertes mvertes added this to the v0.9.x milestone Feb 2, 2021
Copy link
Member

@mvertes mvertes left a comment

Choose a reason for hiding this comment

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

LGTM

@traefiker traefiker merged commit 2e17cfa into traefik:master Feb 4, 2021
mvertes added a commit that referenced this pull request Feb 5, 2021
This is a follow-up of #1017, generalizing the use of reflect.Set
method to set, and possibly overwrite, the concrete value of
interface objects all accross the implementation of operators.
traefiker pushed a commit that referenced this pull request Feb 8, 2021
This is a follow-up of #1017, generalizing the use of reflect.Set
method to set, and possibly overwrite, the concrete value of
interface objects all accross the implementation of operators.
Previous optimized implementation for non-interface objects is
preserved.
mvertes added a commit that referenced this pull request Jul 21, 2021
Map handling builtins getIndexMap and rangeMap had some leftover
code of previous way of emulating interfaces, which was modified
following changes in #1017.

Specific code for interfaceT is removed, as not necessary anymore.
Map builtins are now simplified and more robust.

Fixes #1189.
traefiker pushed a commit that referenced this pull request Jul 26, 2021
Map handling builtins getIndexMap and rangeMap had some leftover
code of previous way of emulating interfaces, which was modified
following changes in #1017.

Specific code for interfaceT is removed, as not necessary anymore.
Map builtins are now simplified and more robust.

Fixes #1189.
traefiker pushed a commit that referenced this pull request Jul 26, 2021
Map handling builtins getIndexMap and rangeMap had some leftover
code of previous way of emulating interfaces, which was modified
following changes in #1017.

Specific code for interfaceT is removed, as not necessary anymore.
Map builtins are now simplified and more robust.

Fixes #1189.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core bug Something isn't working
Projects
None yet
4 participants