gno icon indicating copy to clipboard operation
gno copied to clipboard

fix(gnovm): interfaces cannot embed interface literals

Open omarsy opened this issue 6 months ago • 2 comments

close #4363

omarsy avatar Jun 16 '25 14:06 omarsy

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):
  • [ ] IGNORE the bot requirements for this PR (force green CI check)
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info) 🟢 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: omarsy/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🟢 Requirement satisfied
└── 🟢 If
    ├── 🟢 Condition
    │   └── 🟢 Or
    │       ├── 🟢 At least 1 user(s) of the organization reviewed the pull request (with state "APPROVED")
    │       ├── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🟢 Then
        └── 🟢 And
            ├── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)
            └── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request

Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

  • Any user with comment edit permission

Gno2D2 avatar Jun 16 '25 14:06 Gno2D2

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests.

:loudspeaker: Thoughts on this report? Let us know!

codecov[bot] avatar Jun 16 '25 15:06 codecov[bot]

seems to be a related one, it panics but should not:

package main

type B interface {
	B()
}

type A interface {
	B
	C()
}

type aImpl struct{}

func (aImpl) B() { println(123) }
func (aImpl) C() { println(456) }

func main() {
	var x A = aImpl{}
	x.B()
	x.C()
}

// Output:
// 123
// 456

ltzmaxwell avatar Jun 26 '25 15:06 ltzmaxwell

this looks work, please double check. (for you reference.)

--- a/gnovm/pkg/gnolang/types.go
+++ b/gnovm/pkg/gnolang/types.go
@@ -991,9 +991,8 @@ func (it *InterfaceType) FindEmbeddedFieldType(callerPath string, n Name, m map[
                        if !isUpper(string(n)) && it.PkgPath != callerPath {
                                return nil, false, nil, nil, true
                        }
-                       // a matched name cannot be an embedded interface.
                        if im.Type.Kind() == InterfaceKind {
-                               return nil, false, nil, nil, false
+                               goto RECURSIVE
                        }
                        // match found.
                        tr := []ValuePath{NewValuePathInterface(n)}
@@ -1002,6 +1001,7 @@ func (it *InterfaceType) FindEmbeddedFieldType(callerPath string, n Name, m map[
                        ft := im.Type
                        return tr, hasPtr, rcvr, ft, false
                }
+       RECURSIVE:
                if et, ok := baseOf(im.Type).(*InterfaceType); ok {
                        // embedded interfaces must be recursively searched.
                        trail, hasPtr, rcvr, ft, accessError = et.FindEmbeddedFieldType(callerPath, n, m)

ltzmaxwell avatar Jun 27 '25 03:06 ltzmaxwell

yes thank you indeed it is this. done here https://github.com/gnolang/gno/pull/4379/commits/31c3c0e60d85314c9e06be6da168285cce349f93

omarsy avatar Jun 30 '25 11:06 omarsy

Is this a post Go1.17 thing w/ the introduction of generics? If it works and it's simple, no problem, but arguably there's a case for not supporting it, e.g. Gno2's generics system could diverge from Go.

jaekwon avatar Jul 07 '25 00:07 jaekwon

How about we hold off on this one and call it a known issue? Go1.17 doesn't support this, so it is a generics feature post 1.18, and we aren't targetting Go generics.

w/ go1.17.13
# command-line-arguments
./foo.go:10:2: syntax error: unexpected interface, expecting method or interface name
./foo.go:13:2: syntax error: non-declaration statement outside function body

jaekwon avatar Jul 07 '25 00:07 jaekwon