adding support to object construction with arguments. #259
Conversation
This file contains 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
ben-clayton
suggested changes
Oct 23, 2023
ben-clayton
reviewed
Oct 26, 2023
| MARL_NO_EXPORT inline void borrow(size_t count, const F& f) const; | ||
| template <typename F, typename... Args> | ||
| MARL_NO_EXPORT inline void borrowList(size_t count, const F& f, Args&&... args) const { | ||
| marl::lock lock(storage->mutex); |
There was a problem hiding this comment.
Please also add the static_assert here.
| MARL_NO_EXPORT inline std::pair<Loan, bool> tryBorrow() const; | ||
| template <typename... Args> | ||
| MARL_NO_EXPORT inline std::pair<Loan, bool> tryBorrow(Args&&... args) const { | ||
| Item* item = nullptr; |
| @@ -214,32 +212,65 @@ class BoundedPool : public Pool<T> { | |||
|
|
|||
| // borrow() borrows a single item from the pool, blocking until an item is | |||
| // returned if the pool is empty. | |||
| MARL_NO_EXPORT inline Loan borrow() const; | |||
| template <typename... Args> | |||
| MARL_NO_EXPORT inline Loan borrow(Args&&... args) const { | |||
There was a problem hiding this comment.
Any reason you've inlined these methods into the class? Marl tries to keep the implementations out of the class, so the public interface is easily human parsable. I'd prefer it if these were put back where they were.
| @@ -366,11 +355,6 @@ class UnboundedPool : public Pool<T> { | |||
| MARL_NO_EXPORT inline Storage(Allocator* allocator); | |||
| MARL_NO_EXPORT inline ~Storage(); | |||
| MARL_NO_EXPORT inline void return_(Item*) override; | |||
| // We cannot copy this as the Item pointers would be shared and | |||
There was a problem hiding this comment.
While I see you've moved this up to Storage, I think it would be generally safer if this remained here as well. Please revert.
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.
No description provided.