search

Aeledfyr / deepsize

A rust crate to find the total size of an object, on the stack and on the heap
MIT License
103 stars 19 forks source link

Clarify internal methods #6

Open dtolnay opened 5 years ago

dtolnay commented 5 years ago

Currently two methods of the DeepSizeOf trait are documented as being "internal".

https://github.com/Aeledfyr/deepsize/blob/8c872164c27927ab06a74bf7cabd639fbe118f6d/src/lib.rs#L80-L82

https://github.com/Aeledfyr/deepsize/blob/8c872164c27927ab06a74bf7cabd639fbe118f6d/src/lib.rs#L92-L94

I don't recognize "internal" as being standard Rust terminology. If these methods are intended to be private implementation details of the deepsize crate, implemented and called only from deepsize code, then they should not be rendered in public API documentation.

@Aeledfyr

Aeledfyr commented 5 years ago

I agree that they are implementation details, but they are required to be able to implement DeepSizeOf on a struct that uses heap allocation. The problem is that these methods need mutable state to be able to track references, and that has to be passed somehow. These should only be called from within an implementation of the deep_size_of_children function, but they can't be hidden or private because they are needed to be able to implement the trait. I'm not sure what the best solution to this would be. I might be able to combine or remove one of recurse_deep_size_of and deep_size_of_children to make it more consistent, so that there would only be one implementation method.

dtolnay commented 5 years ago

I think clarifying what "internal" means (as you did in your response) would be sufficient. It seems likely that a user would read that something is "internal" and understand that to mean they are not supposed to implement it themselves.