Skip to content

optimize rtypeMethodX - #98

Merged
visualfc merged 1 commit into
goplus:mainfrom
visualfc:methodx
Aug 4, 2026
Merged

optimize rtypeMethodX#98
visualfc merged 1 commit into
goplus:mainfrom
visualfc:methodx

Conversation

@visualfc

@visualfc visualfc commented Aug 4, 2026

Copy link
Copy Markdown
Member

No description provided.

@fennoai fennoai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review Summary

This PR refactors slice building in both rtypeMethodX implementations from append on a zero-length, capacity-hinted slice to a pre-sized make + indexed assignment. The change is correct and behavior-preserving:

  • Sizing is exact (1+len(ins) for the receiver-prepended in, len(outs) for out), so every index write is provably in bounds.
  • The receiver-prepend logic (in[0] then in[i+1]) matches the previous append order.
  • Empty argument/return cases produce equivalent slices, and no unsafe handling changed.

One minor, low-severity note on loop-variable shadowing is left inline. No correctness, security, performance, or documentation issues were found.

Comment thread rtype.go
Comment thread rtype_llgo.go
@visualfc

visualfc commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

@fennoai

@fennoai

fennoai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review: optimize rtypeMethodX

I reviewed the changes on visualfc:methodx against main (base 0a0fc5f, head b2ebb42).

Summary of changes — identical, well-mirrored edits in rtype.go and rtype_llgo.go:

  1. Parameter rename iindex — frees the name i so it can be reused as the loop index variable in the in/out builders. All references (Method(index), bounds check, methods[index], m.Index = index) are updated consistently.
  2. Slice preallocation — the in/out slices switch from make([]T, 0, cap) + append to make([]T, len) + direct indexed assignment (in[0] = ..., in[i+1] = ..., out[i] = ...). This drops the per-element append bounds/growth checks while producing identical results.

Verification

  • go build ./... passes.
  • ✅ Logic is equivalent: capacity was already exact, so preallocating the length and assigning by index yields the same slice contents and length. The in slice reserves index 0 for the receiver (toType(t)) and fills in[i+1] for each input arg — offsets are correct.
  • ✅ Both files change identically; rtype_llgo.go is under //go:build llgo (not exercised by the default build) but mirrors rtype.go exactly.
  • ℹ️ go vet warnings that appear (xcall.go, method_test.go) are pre-existing and unrelated to this PR.

Conclusion: Correct, cleanly scoped micro-optimization with no behavioral change. Looks good.

@visualfc
visualfc merged commit 98f7dd4 into goplus:main Aug 4, 2026
18 checks passed
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.

1 participant