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

Change unique_span to take a deleter object taking a span, not a template param accepting taking pointer #678

Open
eyalroz opened this issue Sep 10, 2024 · 0 comments

Comments

@eyalroz
Copy link
Owner

eyalroz commented Sep 10, 2024

unique_span deletion - destruction and de-allocation - is more complex than one might think. With trivially-constructible types, we can supposed just use new T[size] to start off and then delete[] eventually. But in fact, the code which needs to run when we perform raw allocation and placement-new construction - a-la std::vector - is more complex, and definitely depends on array size. It is problematic to presume we know how to delete given only a pointer and no size, and unable to make a last-minute call about this. Specifically, the current setup makes it impossible to determine how many placement-destruct's are necessary given a pointer.

Moreover, the bespoke deleter causes unique_span's to be nearly non-interchangeable, because their types are deleter-dependent.

Even though it may cost us a few more bytes, let's:

  • Remove the extra template parameter from `unique_span<T, Deleter> (this means the memory::XXXX:unique_span's will now be just unique_span's)
  • Use a 'universal' deleter type - which can't just be instantiated
  • Offer a default, simple, actual deleter
  • Offer span uspan makers/generators of fixed value and by function.
eyalroz added a commit that referenced this issue Sep 10, 2024
@eyalroz eyalroz self-assigned this Sep 10, 2024
eyalroz added a commit that referenced this issue Sep 10, 2024
* Dropped the deleter template parameter
* Added a deleter member - always of the same type
* The deleter now takes the span
* Added a unique_span generator for non-default-constructible elements
* Dropped the per-memory-space unique_span types - they're all the same type now
* Made the default constructor explicitly zero thing out to avoid spurious deletions of uninitialized / partially uninitialized spans
eyalroz added a commit that referenced this issue Oct 26, 2024
* Dropped the deleter template parameter
* Added a deleter member - always of the same type
* The deleter now takes the span
* Added a unique_span generator for non-default-constructible elements
* Dropped the per-memory-space unique_span types - they're all the same type now
* Made the default constructor explicitly zero thing out to avoid spurious deletions of uninitialized / partially uninitialized spans
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant