Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Add to safe query_interface class #174

Merged
merged 3 commits into from
Oct 20, 2020
Merged

Add to safe query_interface class #174

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a045dc
Add to safe query_interface class
rylev Oct 19, 2020
03622de
Don't assert on failure
rylev Oct 19, 2020
1cc7e00
Merge branch 'master' into safe-query-interface-for-class
rylev Oct 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions macros/support/src/class/class.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ impl Class {
}
});
let debug = self.debug();
let safe_query_interface = self.safe_query_interface();

quote! {
use super::*;
Expand All @@ -159,6 +160,7 @@ impl Class {
#(#methods)*
#add_ref
#query_interface
#safe_query_interface
}
#debug
impl ::std::ops::Drop for #name {
Expand Down Expand Up @@ -217,6 +219,25 @@ impl Class {
}
}
}

fn safe_query_interface(&self) -> TokenStream {
quote! {
pub fn query_interface<T: ::com::Interface>(self: &::std::pin::Pin<::std::boxed::Box<Self>>) -> Option<T> {
let mut result = None;
let hr = unsafe { self.QueryInterface(&T::IID, &mut result as *mut _ as _) };

if ::com::sys::FAILED(hr) {
assert!(
hr == ::com::sys::E_NOINTERFACE || hr == ::com::sys::E_POINTER,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI. In theory, E_POINTER would only be returned if the second parameter is null. The caller can guarantee that this is not the case. In practice, implementations will typically AV by dereferencing the pointer without bothering to check since this is a coding error by the caller. Either way, asserting here is not advised as wonky implementations may return some non-standard result.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since only E_NOINTERFACE is expected it probably doesn't make a lot of sense to return Err(hr) instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MarijnS95 Hmm I'm not sure what errors would be recoverable from here? I think in general, it's just a question of whether it worked or not, but perhaps you're right that for ultimate flexibility we should be returning an HResult on error so that if some implementation returns a non-standard error, the user can inspect that...

"QueryInterface returned non-standard error"
);
return None;
}
debug_assert!(result.is_some(), "Successful call to query_interface yielded a null pointer");
result
}
}
}
}

impl syn::parse::Parse for Class {
Expand Down