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
cmd/compile: static itab construction fails when a type has multiple non-exported methods #24693
Comments
Relatedly: when sorting method sets, we sort non-exported names by their package path. However, we always use We could sort according to the |
We have not yet told people that -p is required when invoking the compiler. I'm not even sure if we do it ourselves consistently everywhere. Almost certainly not. If there's a way to avoid requiring -p that would be nice. That said I agree that the method tables are probably sorted wrong, which is bad. Maybe this is the last straw that pushes us over to requiring -p. Not sure. |
Yeah, I'd really like to avoid requiring Perhaps we could sort non-exported method symbols by package height (i.e., height of the package node in the package dependency DAG) and then package path. The package under compilation is the only package whose path we might not know at the time. It will also always have a unique height within that compilation unit (i.e., at least 1 more than every other package within the CU, by definition), so this ordering should be insensitive to |
Change https://golang.org/cl/105038 mentions this issue: |
Change https://golang.org/cl/105039 mentions this issue: |
Change https://golang.org/cl/105936 mentions this issue: |
This used to be duplicated in methcmp and siglt, because Sig used its own representation for Syms. Instead, just use Syms, and add a (*Sym).Less method that both methcmp and siglt can use. Also, prune some impossible cases purportedly related to blank methods: the Go spec disallows blank methods in interface method sets, and addmethod drops blank methods without actually recording them in the type's method set. Passes toolstash-check. Updates #24693. Change-Id: I24e981659b68504d71518160486989a82505f513 Reviewed-on: https://go-review.googlesource.com/105936 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org>
A package's height is defined as the length of the longest import path between itself and a leaf package (i.e., package with no imports). We can't rely on knowing the path of the package being compiled, so package height is useful for defining a package ordering. Updates #24693. Change-Id: I965162c440b6c5397db91b621ea0be7fa63881f1 Reviewed-on: https://go-review.googlesource.com/105038 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
In reflect.go:genfun, the code to walk method sets in parallel to compute itab entries doesn't take packages into account for finding the right non-exported method. This causes the code below to print
FAIL
instead ofok
, because it selects U.m instead of T.m for the itab.The text was updated successfully, but these errors were encountered: