Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do we need structs in plan::tracing and ProcessedgesWork to be public? #950

Open
qinsoon opened this issue Sep 10, 2023 · 1 comment
Open
Labels
A-interface Area: Interface/API F-question Call For Participation: Unanswered question (need more information) P-normal Priority: Normal.

Comments

@qinsoon
Copy link
Member

qinsoon commented Sep 10, 2023

I noticed these API changes reported in #897. The changed items are exposed as public. But it is probably unnecessary to make them public. We should consider making them private.

Changed items in the public API
===============================
-pub fn mmtk::plan::ObjectsClosure<'a, E>::new(worker: &'a mut mmtk::scheduler::GCWorker<<E as mmtk::scheduler::ProcessEdgesWork>::VM>) -> Self
+pub fn mmtk::plan::ObjectsClosure<'a, E>::new(worker: &'a mut mmtk::scheduler::GCWorker<<E as mmtk::scheduler::ProcessEdgesWork>::VM>, bucket: mmtk::scheduler::WorkBucketStage) -> Self
...
-pub fn mmtk::scheduler::ProcessEdgesWork::new(edges: alloc::vec::Vec<<<Self as mmtk::scheduler::ProcessEdgesWork>::VM as mmtk::vm::VMBinding>::VMEdge>, roots: bool, mmtk: &'static mmtk::MMTK<Self::VM>) -> Self
+pub fn mmtk::scheduler::ProcessEdgesWork::new(edges: alloc::vec::Vec<<<Self as mmtk::scheduler::ProcessEdgesWork>::VM as mmtk::vm::VMBinding>::VMEdge>, roots: bool, mmtk: &'static mmtk::MMTK<Self::VM>, bucket: mmtk::scheduler::WorkBucketStage) -> Self
@qinsoon qinsoon added F-question Call For Participation: Unanswered question (need more information) A-interface Area: Interface/API labels Sep 10, 2023
@wks
Copy link
Collaborator

wks commented Sep 11, 2023

It's likely a bug. It should be enough to keep them pub(crate).

VM bindings don't need to depend on the ProcessEdgesWork trait in order to work. See #599

@qinsoon qinsoon added the P-normal Priority: Normal. label Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-interface Area: Interface/API F-question Call For Participation: Unanswered question (need more information) P-normal Priority: Normal.
Projects
None yet
Development

No branches or pull requests

2 participants