Skip to content

add core static value member support#638

Merged
xushiwei merged 4 commits into
goplus:mainfrom
KurodaKayn:static-value-member-core
Jul 1, 2026
Merged

add core static value member support#638
xushiwei merged 4 commits into
goplus:mainfrom
KurodaKayn:static-value-member-core

Conversation

@KurodaKayn

Copy link
Copy Markdown
Contributor

Summary

  • Add the core static value member path for named types.
  • This introduces NewStaticMember so package-level const/var objects can be associated with a named type and resolved through normal CodeBuilder.Member handling on TypeType receivers.
  • Part of support static value members for named types #637

Changes

  • Add NewStaticMember for registering static const/var members on named types.
  • Resolve registered static value members from cb.Typ(T).MemberVal(...).
  • Resolve registered static var members from cb.Typ(T).MemberRef(...) so assignable operations like inc/dec work.
  • Keep static methods and static value members in the same member namespace by rejecting collisions.
  • Normalize generic instantiated named types to Origin() for static member lookup, so Box[int].name can resolve members registered on Box[T].

Testing

  • go test -run 'TestStaticMember|TestStaticMethod' ./...
  • go test ./...
  • git diff --check

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@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.

Summary

The PR introduces a clean, mutex-protected registry for associating const/var objects with named types. Generic-type origin normalization via staticMemberOrigin is handled consistently, and the integration into Member() correctly places static lookup ahead of regular method/field resolution without breaking the existing isType && kind != MemberMethod guard.

Three inline findings cover misleading panic messages and dead code. Two non-inline findings cover a missing symmetric test and the global registry lifetime.


Missing test — NewStaticMethod after NewStaticMember conflict (xgo_test.go)

The new guard added to NewStaticMethod (if lookupStaticMember(typ, name) != nil { ... }) has no direct test coverage. TestStaticMemberMethodConflict exercises the reverse direction only (static member after static method). A symmetric test would prevent regressions on this new guard.


Global registry lifetime (func_ext.go, staticMemberValues)

Unlike NewStaticMethod, which stores entries directly on the *types.Named via the Go type system (and is collected when the type is GC'd), staticMemberValues is a process-global map that never evicts entries. In long-lived processes that build many packages (e.g. a language server or batch compile server), this retains *types.Named pointers and their types.Object values indefinitely. Consider scoping the registry to a Package/builder instance, or documenting the lifetime trade-off with a comment on the global.

Comment thread func_ext.go Outdated
Comment thread func_ext.go Outdated
Comment thread func_ext.go Outdated
@KurodaKayn KurodaKayn force-pushed the static-value-member-core branch 2 times, most recently from 62b592e to b84da73 Compare June 24, 2026 03:36
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.68%. Comparing base (4e08570) to head (1bf0f5a).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #638      +/-   ##
==========================================
+ Coverage   93.63%   93.68%   +0.04%     
==========================================
  Files          29       29              
  Lines        7167     7221      +54     
==========================================
+ Hits         6711     6765      +54     
  Misses        388      388              
  Partials       68       68              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KurodaKayn KurodaKayn force-pushed the static-value-member-core branch from c9a7472 to 823891a Compare June 24, 2026 14:21
Comment thread codebuild.go Outdated
Comment thread codebuild_test.go
@KurodaKayn KurodaKayn force-pushed the static-value-member-core branch from 823891a to c9a1de6 Compare June 28, 2026 06:05
@KurodaKayn KurodaKayn requested a review from xushiwei June 28, 2026 06:15
@xushiwei

Copy link
Copy Markdown
Member

Does it seem like simply upgrading NewStaticMethod/TyStaticMethod to NewStaticMember/TyStaticMember would solve the problem?

@xushiwei

Copy link
Copy Markdown
Member

In addition, XGo's engineering guidelines discourage defensive programming, such as name == "" (unless it would have a very significant negative impact). This does not add detection code but is explained in the documentation.

@KurodaKayn KurodaKayn force-pushed the static-value-member-core branch from c9a1de6 to 521638e Compare June 29, 2026 15:48
@KurodaKayn

Copy link
Copy Markdown
Contributor Author

Does it seem like simply upgrading NewStaticMethod/TyStaticMethod to NewStaticMember/TyStaticMember would solve the problem?

You're right. Now I just upgraded NewStaticMethod/TyStaticMethod to NewStaticMember/TyStaticMembe.

Besides, I kept TyStaticMethod as a deprecated compatibility layer, with NewStaticMethod delegating to NewStaticMember.

Could you confirm which direction you prefer here: should we keep this compatibility layer for existing users, or remove TyStaticMethod / NewStaticMethod directly and fully switch to TyStaticMember / NewStaticMember?

@xushiwei

Copy link
Copy Markdown
Member

Could you confirm which direction you prefer here: should we keep this compatibility layer for existing users, or remove TyStaticMethod / NewStaticMethod directly and fully switch to TyStaticMember / NewStaticMember?

remove them directly

@KurodaKayn KurodaKayn force-pushed the static-value-member-core branch from 521638e to 1bf0f5a Compare June 30, 2026 07:44
@KurodaKayn

Copy link
Copy Markdown
Contributor Author

remove them directly

Okay, it's clean now

@xushiwei xushiwei merged commit 535011f into goplus:main Jul 1, 2026
20 checks passed
@KurodaKayn KurodaKayn deleted the static-value-member-core branch July 1, 2026 06:47
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