Skip to content

fix(gnovm): ignore blank fields in struct equality and map keys#5784

Open
omarsy wants to merge 4 commits into
gnolang:masterfrom
omarsy:codex/fix-struct-blank-field-equality
Open

fix(gnovm): ignore blank fields in struct equality and map keys#5784
omarsy wants to merge 4 commits into
gnolang:masterfrom
omarsy:codex/fix-struct-blank-field-equality

Conversation

@omarsy

@omarsy omarsy commented Jun 5, 2026

Copy link
Copy Markdown
Member

Summary

Fixes Go 1.17 spec mismatches in Gno struct equality and map keys.

The Go spec says struct equality compares corresponding non-blank fields. Gno was also comparing fields named _, so values like:

struct{ _ int; X int }{1, 2} == struct{ _ int; X int }{3, 2}

incorrectly evaluated to false.

Gno also encoded blank struct fields into map keys, so a lookup such as:

m := map[struct{ _ int; X int }]string{{1, 2}: "a"}
m[struct{ _ int; X int }{3, 2}]

missed the existing entry even though the keys are equal under the Go spec.

This updates StructKind equality and struct map-key encoding to skip blank fields while preserving existing comparability checks. A companion negative test verifies that blank fields still count for comparability, so a struct containing _ []int remains non-comparable.

Spec reference:
https://go.googlesource.com/go/+/refs/tags/go1.17.5/doc/go_spec.html#Comparison_operators

Tests

go test ./gnovm/pkg/gnolang -run 'TestFiles/types/cmp_struct_[hi]' -count=1 -v
go test ./gnovm/pkg/gnolang -run 'TestFiles/types/cmp_struct' -count=1
go test ./gnovm/pkg/gnolang -run 'TestFiles/(map48|types/cmp_struct_h|types/cmp_struct_i)' -count=1 -v
go test ./gnovm/pkg/gnolang -run 'TestFiles/map4' -count=1
go test ./gnovm/pkg/gnolang -run 'TestComputeMapKey' -count=1
git diff --check

Note: a full go test ./gnovm/pkg/gnolang -count=1 was started before the map-key follow-up but did not complete after several minutes, so it was stopped.

AI Disclosure

This PR was prepared with AI assistance.

@Gno2D2

Gno2D2 commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

🛠 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
    │       ├── 🟢 User davd-gzl already reviewed PR 5784 with state APPROVED
    │       ├── 🔴 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🟢 Then
        └── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)

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

@omarsy omarsy changed the title [codex] fix(gnovm): ignore blank fields in struct equality fix(gnovm): ignore blank fields in struct equality Jun 5, 2026
@omarsy omarsy changed the title fix(gnovm): ignore blank fields in struct equality fix(gnovm): ignore blank fields in struct equality and map keys Jun 5, 2026
…blank-field-equality

# Conflicts:
#	gnovm/pkg/gnolang/op_binary.go
#	gnovm/tests/files/map48.gno
@codecov

codecov Bot commented Jun 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@omarsy omarsy marked this pull request as ready for review June 8, 2026 15:12
@Gno2D2 Gno2D2 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Jun 8, 2026
@omarsy omarsy requested review from a team, ltzmaxwell and thehowl June 8, 2026 18:23

@davd-gzl davd-gzl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It miss the test where all keys are blank: struct{ _, _ int }
This is a behavioral break for any persisted code using blank key. This should not be a problem

@Gno2D2 Gno2D2 removed the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Jun 8, 2026
@thehowl thehowl added the a/vm GnoVM, Security, Runtime team label Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a/vm GnoVM, Security, Runtime team 📦 🤖 gnovm Issues or PRs gnovm related

Projects

Development

Successfully merging this pull request may close these issues.

4 participants