Incorrect relations between interfaces can cause realm panics (cont.)
Description
After merging https://github.com/gnolang/gno/pull/4861 a new issue is introduced, because it compares only the number of functions in the types rather than matching their names precisely.
Example: https://gist.github.com/kirillt/5fa37c1fa85b1654adace34724ebff25
this is what it behaves in master, seems not a bug, nor related to #4861:
package main
type A interface {
F()
}
type B interface {
X()
G()
}
type T struct{}
func (T) F() {}
func (T) G() {}
func (T) X() {}
func main() {
var a A = T{}
var b B = T{}
println(b == a)
println(a == b)
}
// Error:
// main/cmp_iface_9.gno:24:10-16: main.B does not implement main.A (missing method F)
I reached out to the issue reporter to confirm
@ltzmaxwell reporter's reply
after reviewing the latest changes, we still have concerns about the correctness of the proposed fix. It appears that the fix focuses on counting the number of functions to determine “specificity,” but this approach doesn’t capture the actual subtype relationship involved in the original issue.
To clarify our position:
- “Less specific” in this context means “is a subtype.”
- Counting functions can only be used as a quick negative check (a subtype cannot have more functions than its parent), but it does not prove that one type is a subtype of another.
- Two types that share no overlapping functions remain incomparable, regardless of function count. In such cases, shouldSwapOnSpecificity should not return true for either ordering of the arguments.
- The test case previously shared should ideally be turned into a unit test to verify that the function behaves correctly for incomparable types.
Could you please clarify why you believe the current fix addresses the underlying problem and why the additional adjustments we suggested would not be necessary? This would help us confirm the intended logic and ensure accurate validation.
Thank you for reporting!
Counting functions can only be used as a quick negative check (a subtype cannot have more functions than its parent), but it does not prove that one type is a subtype of another.
Two types that share no overlapping functions remain incomparable, regardless of function count. In such cases, shouldSwapOnSpecificity should not return true for either ordering of the arguments.
It’s true that simply comparing method counts is not enough to "prove that one type is a subtype of another". However, that responsibility belongs to the downstream logic—specifically checkAssignableTo, which takes xt (the source type) and dt (the destination type) and determines whether xt is assignable to dt, ensuring they are comparable. per the Go spec:
In any comparison, the first operand must be assignable to the type of the second operand, or vice versa.(https://go.dev/ref/spec#Comparison_operators)
shouldSwapOnSpecificity is a quick negative check used to rule out obviously incompatible pairs of xt and dt. e.g. in below case, type of A won't be assignable to type of B.
type A interface {
F()
}
type B interface {
X()
G()
}
Anyway, this PR improves the error messages to make them more in line with Go’s style: https://github.com/gnolang/gno/pull/4936
The test case previously shared should ideally be turned into a unit test to verify that the function behaves correctly for incomparable types.
This is a filetest(efficient for VM test) that effectively illustrates the test case: https://github.com/gnolang/gno/pull/4936/commits/7663fdca16fe025e89c034b2e9fda2be944711d4