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.
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
Provide an explicit
ComObject<T>
type that represents a heap-allocated COM object #3043Provide an explicit
ComObject<T>
type that represents a heap-allocated COM object #3043Changes from 9 commits
0e20d29
2dbb95c
dbcb9cd
95833f3
fa0ab0b
4e4addb
6981799
9f7a58b
ec29c03
f72fd9e
326f340
13f251e
15d5851
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Taking a step back, how useful is
ComObject
outside the context of object construction? Sinceget_mut
necessarily can't be used without exclusive ownership, it seems as ifComObject
would only ever be used for object construction. If that is the case, I'm wondering whether anything can be simplified about this design. It is not overly complicated as it is, just something to consider.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.
You can of course still use
get
to retrieve a synchronization primitive likeRwLock
stored within the implementation and go from there. So this could well be used during the life cycle of the object. Is that how you see it being used?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.
It's useful outside of just construction. A common pattern in DWriteCore is that we create a bunch of COM objects, and hand out interfaces to the client process, but we also need to keep strongly-typed references to the contents. Right now I'm doing that with the equivalent that is in the old
com
crate, and havingComObject
inwindows_core
gets me that much closer to converting DWriteCore to use the Windows crates.get_mut() itself is mainly useful during construction. You create a ComObject, then get access to its guts while you're still the only reference holder. You could get the same effect with RefCell, but since we already have the reference count in ComObject, why not use it? It's basically free.
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.
Seems like an implementation detail - does it need to be public?
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.
Seems like an implementation detail - does it need to be public?
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.
How useful is this in practice? Seems like it wouldn't be much harder just writing
as_interface().to_owned()
.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.
It's just a convenience. I think we'll need to use this stuff a bunch to see what's the most useful.