refactor(gnovm): derive IsAssignable from NameSources, drop StaticBlock.UnassignableNames - #6089
Open
ltzmaxwell wants to merge 4 commits into
Open
refactor(gnovm): derive IsAssignable from NameSources, drop StaticBlock.UnassignableNames#6089ltzmaxwell wants to merge 4 commits into
ltzmaxwell wants to merge 4 commits into
Conversation
…bleNames StaticBlock.UnassignableNames only ever held package-level func decl names -- the same fact Reserve already records as NSFuncDecl in NameSources, aligned by the same local index. Answer IsAssignable from the NameSource type instead and delete the duplicate slice: one less serialized field, and an O(1) check instead of an O(n) scan. The amino slot is retired with a blank amino:"reserved" field, the mechanism introduced for Externs in gnolang#5301, so field numbers are unchanged and old encoded data carrying field 8 is skipped on decode.
Collaborator
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
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) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
Avoids confusion with type assignability (checkAssignableTo, mustAssignableTo): this predicate is about the name's object kind (func decl / uverse name vs. assignable), not about types.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
StaticBlock.UnassignableNames(introduced in #3198) only ever had one writer: the non-method*FuncDeclcase ininitStaticBlocks, which appended the func decl's name immediately afterReserve(false, nx, n, NSFuncDecl, -1)recorded the same fact inNameSources. Despite its general-sounding name, the slice never held any other kind of unassignable name — constants are const-folded (and tracked inConsts), type names fold toconstTypeExpr, and uverse names are refused by an explicit branch inIsAssignable.This PR deletes the field and answers
IsAssignablefrom the declaring block'sNameSourcesinstead:NameSources[idx].Type != NSFuncDecl— the same idiom already used forNSTypeDecllookups inpreprocess.go. Indexing by aGetLocalIndexresult is safe becauseDefine2enforcesNumNames == len(NameSources).Changes
nodes.go— drop theUnassignableNamesfield (slot retired with a blank_ struct{} \amino:"reserved"`field, the mechanism from #5301, so field numbers are unchanged and old encoded data carrying field 8 is skipped on decode); rewriteIsAssignableto an O(1)NameSourceslookup instead of an O(n) scan, now shaped exactly like its siblingGetIsConst`.preprocess.go— drop the duplicate append ininitStaticBlocks2.gnolang.proto,pb3_gen.go— regenerated withmisc/genproto2(emitsreserved 8;).gnovm/adr/pr6089_remove_unassignable_names.md— ADR.Verification
go test ./gnovm/pkg/gnolang/ -short: failure set identical to pristinemasteron the same machine (10 pre-existing go/types wording goldens, diffed test-by-test) — nothing new.go test ./gno.land/pkg/sdk/vm/ -run Gas,go test ./gno.land/pkg/integration/ -run TestTestdata,go test ./tm2/pkg/amino/...: all pass.SetBlockNode's backend write is a TODO — so this is schema hygiene, not a live migration).Possible follow-ups (out of scope)
IsAssignableAt(store, path): theAssignStmtcall site already has a resolvedValuePath, so the name walk could become O(depth) pointer hops, mirroringGetIsConstAt. Needs care around blank identifiers and path-refill timing.ConstsintoNameSourcesthe same way, retiring the last parallel name-list and its// TODO consider merging with Names.Context: grew out of review discussion on #6083, where the addressability check leans on
IsAssignableand the field's name suggested a completeness ("all unassignable names") it never had.AI-assisted: implementation and ADR drafted with Claude, reviewed by the author.
🤖 Generated with Claude Code