-
Notifications
You must be signed in to change notification settings - Fork 17
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
Placeholder accessors can be default constructed #18
base: master
Are you sure you want to change the base?
Conversation
* Placeholder accessor constructor can optionally not take memobj * Handler can specify the buffer later on
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proposal looks good, though I think this would benefit from having a member function for querying whether a placeholder accessor requires a buffer to be provided to require()
, for example accessor::has_buffer()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some additional comments.
placeholder_accessors/index.md
Outdated
The accessor API features constructors that don't take the handler parameter | ||
and/or memory object as a constructor. Accessors can then be default | ||
constructed, and the memory object can be assigned later when registering | ||
the accessor in the command group. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should describe the additional constructors which need to be added to allow deferred initialization of the memory object and handler.
@@ -192,6 +195,12 @@ The handler gains a new method, | |||
that registers the requirements of the placeholder accessor on the given |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we wish to support the option for constructing a regular accessor from a placeholder accessor (i.e. get_access
) then this interface should also be extended to allow you to specify a memory object. We should also specify that an exception should be thrown if get_access
is called without providing a memory object on a placeholder accessor which doesn't have a memory object already.
On a side note, perhaps we should consider changing the naming of this member function, get_access
could be confused with the regular get_access
member functions.
#21) * Providing explicit documentation for throwing an exception when no buffer exists. * Removed implementation details and provided interface specification only. * Applied fixes to Gordon's review * Changed has_buffer to be a const function * Changed wording for the placeholder accessor buffer
I believe #124 offers a suitable alternative to this. |
Placeholder accessor constructor can optionally not take memobj
Handler can specify the buffer later on