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

runtime: propose default alg->equals and alg->hash when nil #7447

Closed
gopherbot opened this issue Mar 3, 2014 · 10 comments
Closed

runtime: propose default alg->equals and alg->hash when nil #7447

gopherbot opened this issue Mar 3, 2014 · 10 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@gopherbot
Copy link

by carlchatfield:

Hello,

I have been grappling with defining a suitable Alg for types created at runtime.
The problem is that for each comparable type not stored in continuous (memcmp-able)
memory, specific code is generated by the compiler to perform the equals/hash operations.

To create types at runtime, e.g. using reflect, emitting type specific equals/hash code
is difficult because the code must somehow reference the type.
After much grappling, I have come to the conclusion that having a default equals and
hash function may be the simplest solution.

Types not compiled into the binary can only ever exist in the runtime as interfaces. If
a type assertion from Value.Interface() to X existed in the code, X would also exist in
the binary. Note that if the type is compiled into the binary, there is no need to
generate an Alg.

This means types created at runtime are only ever compared/hashed indirectly through
runtime.interhash() and runtime.interequal(). In each case, there is exactly one spot in
the code where the underlying type->alg->equals/hash is called, ifaceeq1 and
ifacehash1 of iface.goc. Note that at both points the type of the underlying memory is
known, making generic algorithms feasible.

Therefore, IMHO a clean solution to the problem is to add an extra case here to make a
generic equals/hash when the corresponding field is nil.

If this sounds reasonable I am prepared to try my hand at it.
@gopherbot
Copy link
Author

Comment 1 by carlchatfield:

My apologies to whoever gets spammed with these, the title should read 
pkg/runtime: Propose default alg->equals and alg->hash when nil
Not sure what happened

@ianlancetaylor
Copy link
Contributor

Comment 2:

Sounds reasonable to me, but as described at http://golang.org/doc/contribute.html you
should raise this issue on the golang-dev mailing list.  Thanks.

@rsc
Copy link
Contributor

rsc commented Mar 3, 2014

Comment 3:

Labels changed: added release-none.

Status changed to Thinking.

@bradfitz
Copy link
Contributor

bradfitz commented Mar 5, 2014

Comment 4:

Carl sent https://golang.org/cl/70200056

@griesemer
Copy link
Contributor

Comment 5:

Labels changed: added repo-main.

@ysmolski
Copy link
Member

ysmolski commented Nov 6, 2018

Excuse me, @0xfaded. Do you think we should we keep this issue open? I see it as some experiment that happened long time ago without any outcome. Do you think that it should be applied to existing runtime?

@ysmolski ysmolski added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed Thinking labels Nov 6, 2018
@0xfaded
Copy link
Contributor

0xfaded commented Nov 6, 2018

Yeah, please close this. I guess it was somehow transferred from the old bug tracker.

@ysmolski
Copy link
Member

ysmolski commented Nov 6, 2018

Closing per @0xfaded comment.

@ysmolski ysmolski closed this as completed Nov 6, 2018
@randall77
Copy link
Contributor

We don't need to do this anymore. The algorithms can be closures, which close over the type. For example, reflect uses closures to make eq and hash functions.

@ysmolski
Copy link
Member

ysmolski commented Nov 6, 2018

Thanks Keith for the explanation!

@golang golang locked and limited conversation to collaborators Nov 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

8 participants