Skip to content

Commit

Permalink
fix(gnovm): Resolve RefValues before using as Objects (#2454)
Browse files Browse the repository at this point in the history
Closes #2449.

I believe the bug was introduced by #2255. Previously, if the type of
`tv.V` in `GetFirstObject` was a `PointerValue`, it the `Base` would
have been nil in the scenario producing this bug. Now that `Base` can
never be nil for a `PointerValue`, the nil base check has been removed.
We do however need to account for a new scenario where the `Base` is a
`RefValue`, a type that does implement the `Object` interface. In the
case where the base is a `RefValue`, first resolve it by calling
`store.GetObject`.

<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [x] Added new tests, or not needed, or not feasible
- [x] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [x] Updated the official documentation or not needed
- [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [x] Added references to related issues and PRs
- [x] Provided any useful hints for running manual tests
- [x] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
  • Loading branch information
deelawn authored Jul 2, 2024
1 parent 0572de4 commit 3b42391
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 4 deletions.
2 changes: 1 addition & 1 deletion gnovm/pkg/gnolang/ownership.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func (tv *TypedValue) GetFirstObject(store Store) Object {
// something in it; in that case, ignore the base. That will
// likely require maybe a preparation step in persistence
// ( or unlikely, a second type of ref-counting).
return cv.Base.(Object)
return cv.GetBase(store)
case *ArrayValue:
return cv
case *SliceValue:
Expand Down
4 changes: 1 addition & 3 deletions gnovm/pkg/gnolang/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,6 @@ const (
PointerIndexNative = -3 // Base is *NativeValue.
)

/*
func (pv *PointerValue) GetBase(store Store) Object {
switch cbase := pv.Base.(type) {
case nil:
Expand All @@ -204,10 +203,9 @@ func (pv *PointerValue) GetBase(store Store) Object {
case Object:
return cbase
default:
panic("should not happen")
panic(fmt.Sprintf("unexpected pointer base type %T", cbase))
}
}
*/

// cu: convert untyped; pass false for const definitions
// TODO: document as something that enables into-native assignment.
Expand Down
65 changes: 65 additions & 0 deletions gnovm/tests/files/issue-2449.gno
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// PKGPATH: gno.land/r/evt_test
package evt_test

type Event struct {
name string
}

var deletionEvents = []*Event{
{name: "event1"},
{name: "event2"},
{name: "event3"},
{name: "event4"},
}

var insertEvents = []*Event{
{name: "event1"},
{name: "event2"},
}

var appendEvents = []*Event{
{name: "event1"},
}

func DelEvent(name string) {
for i, event := range deletionEvents {
if event.name == name {
deletionEvents = append(deletionEvents[:i], deletionEvents[i+1:]...)
return
}
}
}

func InsertEvent(name string) {
insertEvents = append(insertEvents[:1], append([]*Event{{name: name}}, insertEvents[1:]...)...)
}

func AppendEvent(name string) {
appendEvents = append(appendEvents, &Event{name: name})
}

func printEvents(events []*Event) {
for _, event := range events {
println(event.name)
}
}

func main() {
DelEvent("event2")
InsertEvent("event1.5")
AppendEvent("event2")

printEvents(deletionEvents)
printEvents(insertEvents)
printEvents(appendEvents)
}

// Output:
// event1
// event3
// event4
// event1
// event1.5
// event2
// event1
// event2

0 comments on commit 3b42391

Please sign in to comment.