Skip to content

Allow allocators in testing (take 2)#2940

Merged
IntegratedQuantum merged 6 commits intoPixelGuys:masterfrom
Argmaster:allocators-for-testing
Apr 20, 2026
Merged

Allow allocators in testing (take 2)#2940
IntegratedQuantum merged 6 commits intoPixelGuys:masterfrom
Argmaster:allocators-for-testing

Conversation

@Argmaster
Copy link
Copy Markdown
Collaborator

This pull request default initializes main.stackAllocator and main.globalAllocator to testing allocator.

Resolves: #1286

@Argmaster Argmaster marked this pull request as ready for review April 19, 2026 12:48
@Argmaster Argmaster moved this to Easy to Review in PRs to review Apr 19, 2026
Copy link
Copy Markdown
Member

@IntegratedQuantum IntegratedQuantum left a comment

Choose a reason for hiding this comment

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

Did you check if they are usable in testing?

Comment thread src/utils/heap.zig Outdated
@IntegratedQuantum IntegratedQuantum moved this from Easy to Review to In review in PRs to review Apr 19, 2026
Comment thread src/utils/heap.zig Outdated
Comment thread src/main.zig Outdated
@Argmaster
Copy link
Copy Markdown
Collaborator Author

One issue with that is that there is no difference between deallocating something with stackAllocator and globalAllocator in testing, right?
Also, is the testing allocator reset every test or just at the very end?

@IntegratedQuantum
Copy link
Copy Markdown
Member

Yes, that is true, though that is technically also possible (to some degree) at runtime, since the stackAllocator uses the globalAllocator as a fallback. If it's trivial to fix, go for it, otherwise I'd say it's not worth the effort.

Also, is the testing allocator reset every test or just at the very end?

It leak-checks every test.

@Argmaster
Copy link
Copy Markdown
Collaborator Author

Argmaster commented Apr 19, 2026

Yes, that is true, though that is technically also possible (to some degree) at runtime, since the stackAllocator uses the globalAllocator as a fallback. If it's trivial to fix, go for it, otherwise I'd say it's not worth the effort.

Hm, then its not trivial to fix I think and its not really critical tbh. IMO out of scope for this PR.

@IntegratedQuantum
Copy link
Copy Markdown
Member

Alright, then I'm still waiting for an answer to my earlier question:

Did you check if they are usable in testing?

@Argmaster
Copy link
Copy Markdown
Collaborator Author

Did you check if they are usable in testing?

Right, I didn't push it;
I added a small test which I think should be sufficient:

test "allocators" {
	const allocation1 = stackAllocator.create(u64);
	stackAllocator.destroy(allocation1);

	const allocation2 = globalAllocator.create(u64);
	globalAllocator.destroy(allocation2);
}

Comment thread src/main.zig Outdated
Co-authored-by: IntegratedQuantum <43880493+IntegratedQuantum@users.noreply.github.com>
@IntegratedQuantum IntegratedQuantum merged commit d28595a into PixelGuys:master Apr 20, 2026
1 check passed
codemob-dev pushed a commit to codemob-dev/Cubyz that referenced this pull request Apr 20, 2026
This pull request default initializes `main.stackAllocator` and
`main.globalAllocator` to testing allocator.

Resolves: PixelGuys#1286

---------

Co-authored-by: IntegratedQuantum <43880493+IntegratedQuantum@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Allocators in testing

2 participants