Skip to content

rlp: return Iterator as non-pointer#33818

Merged
fjl merged 1 commit into
ethereum:masterfrom
fjl:rlp-iterator-non-pointer
Feb 11, 2026
Merged

rlp: return Iterator as non-pointer#33818
fjl merged 1 commit into
ethereum:masterfrom
fjl:rlp-iterator-non-pointer

Conversation

@fjl
Copy link
Copy Markdown
Contributor

@fjl fjl commented Feb 11, 2026

Most uses of the iterator are like this:

it, _ := rlp.NewListIterator(data)
for it.Next() {
    do(it.Value())
}

This doesn't require the iterator to be a pointer and it's better to have it stack-allocated. AFAIK the compiler cannot prove it is OK to stack-allocate when it is returned as a pointer because the methods of Iterator use pointer receiver and also mutate the object.

The iterator type was not exported until very recently, so I think it is still OK to change this API.

Most uses of the iterator are like this:

    it, _ := rlp.NewListIterator(data)
    for it.Next() {
        do(it.Value())
    }

This doesn't require the iterator to be a pointer and it's better
to have it stack-allocated.
Copy link
Copy Markdown
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

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

SGTM

@fjl fjl merged commit 341907c into ethereum:master Feb 11, 2026
10 of 11 checks passed
@fjl fjl added this to the 1.17.0 milestone Feb 11, 2026
sduchesneau pushed a commit to streamingfast/go-ethereum that referenced this pull request Apr 7, 2026
Most uses of the iterator are like this:

    it, _ := rlp.NewListIterator(data)
    for it.Next() {
        do(it.Value())
    }

This doesn't require the iterator to be a pointer and it's better to
have it stack-allocated. AFAIK the compiler cannot prove it is OK to
stack-allocate when it is returned as a pointer because the methods of
`Iterator` use pointer receiver and also mutate the object.

The iterator type was not exported until very recently, so I think it is
still OK to change this API.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants