Skip to content

Commit

Permalink
Bug 1779326 - Handle a few more native types in rust-xpidl, r=xpcom…
Browse files Browse the repository at this point in the history
…-reviewers,kmag

This makes the logic for the rust type line up a bit more with the C++
logic for existing types, and adds support for 'char' and 'char16_t'
native types (for 'charPtr').

This specifically enables `nsIInputStream::Read` to be used from Rust.

Differential Revision: https://phabricator.services.mozilla.com/D152715
  • Loading branch information
mystor committed Jul 27, 2022
1 parent 29842b8 commit c1e984d
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 35 deletions.
2 changes: 1 addition & 1 deletion docs/writing-rust-code/xpcom.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl MyObserver {
&self,
_subject: *const nsISupports,
_topic: *const c_char,
_data: *const i16,
_data: *const u16,
) -> nsresult {
self.ran.store(true, Ordering::SeqCst);
nserror::NS_OK
Expand Down
4 changes: 2 additions & 2 deletions services/common/app_services_logger/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,13 @@ fn ensure_observing_shutdown() {
struct InitShutdownObserver {}

impl ShutdownObserver {
xpcom_method!(observe => Observe(_subject: *const nsISupports, topic: *const c_char, _data: *const i16));
xpcom_method!(observe => Observe(_subject: *const nsISupports, topic: *const c_char, _data: *const u16));
/// Remove our shutdown observer and clear the map.
fn observe(
&self,
_subject: &nsISupports,
topic: *const c_char,
_data: *const i16,
_data: *const u16,
) -> Result<(), nsresult> {
LOGGERS_BY_TARGET.write().unwrap().clear();
if let Some(service) = xpcom::services::get_ObserverService() {
Expand Down
4 changes: 2 additions & 2 deletions toolkit/components/glean/src/init/upload_pref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl UploadPrefObserver {
&self,
_subject: *const nsISupports,
topic: *const c_char,
pref_name: *const i16,
pref_name: *const u16,
) -> nserror::nsresult {
let topic = CStr::from_ptr(topic).to_str().unwrap();
// Conversion utf16 to utf8 is messy.
Expand All @@ -69,7 +69,7 @@ impl UploadPrefObserver {
// cargo-culted from https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/security/manager/ssl/cert_storage/src/lib.rs#1606-1612
// (with a little transformation)
let len = (0..).take_while(|&i| *pref_name.offset(i) != 0).count(); // find NUL.
let slice = std::slice::from_raw_parts(pref_name as *const u16, len);
let slice = std::slice::from_raw_parts(pref_name, len);
let pref_name = match String::from_utf16(slice) {
Ok(name) => name,
Err(_) => return NS_ERROR_FAILURE,
Expand Down
2 changes: 1 addition & 1 deletion toolkit/components/glean/src/init/user_activity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl UserActivityObserver {
&self,
_subject: *const nsISupports,
topic: *const c_char,
_data: *const i16,
_data: *const u16,
) -> nserror::nsresult {
match CStr::from_ptr(topic).to_str() {
Ok("user-interaction-active") => self.handle_active(),
Expand Down
2 changes: 1 addition & 1 deletion toolkit/components/xulstore/src/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ impl ProfileChangeObserver {
&self,
_subject: *const nsISupports,
_topic: *const c_char,
_data: *const i16,
_data: *const u16,
) -> nsresult {
update_profile_dir();
NS_OK
Expand Down
80 changes: 54 additions & 26 deletions xpcom/idl-parser/xpidl/xpidl.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,8 @@ def rustType(self, calltype, shared=False, const=False):
Builtin("double", "double", "libc::c_double", True, False),
Builtin("char", "char", "libc::c_char", True, False),
Builtin("string", "char *", "*const libc::c_char", False, False),
Builtin("wchar", "char16_t", "i16", False, False),
Builtin("wstring", "char16_t *", "*const i16", False, False),
Builtin("wchar", "char16_t", "u16", False, False),
Builtin("wstring", "char16_t *", "*const u16", False, False),
]

builtinMap = {}
Expand Down Expand Up @@ -639,41 +639,69 @@ def nativeType(self, calltype, const=False, shared=False):
def rustType(self, calltype, const=False, shared=False):
# For the most part, 'native' types don't make sense in rust, as they
# are native C++ types. However, we can support a few types here, as
# they're important.
# they're important and can easily be translated.
#
# NOTE: This code doesn't try to perfectly match C++ constness, as
# constness doesn't affect ABI, and raw pointers are already unsafe.

if self.modifier not in ["ptr", "ref"]:
raise RustNoncompat("Rust only supports [ref] / [ptr] native types")

prefix = "*mut " if "out" in calltype else "*const "
if "out" in calltype and self.modifier == "ptr":
prefix += "*mut "
if shared:
if calltype != "out":
raise IDLError(
"[shared] only applies to out parameters.", self.location
)
const = True

if self.specialtype == "nsid":
if "element" in calltype:
if self.isPtr(calltype):
raise IDLError(
"Array<nsIDPtr> not yet supported. "
"File an XPConnect bug if you need it.",
self.location,
)
return self.nativename
return prefix + self.nativename
if self.specialtype in ["cstring", "utf8string"]:
if "element" in calltype:
return "::nsstring::nsCString"
return prefix + "::nsstring::nsACString"
if self.specialtype == "astring":
if "element" in calltype:
return "::nsstring::nsString"
return prefix + "::nsstring::nsAString"
# 'in' nsid parameters should be made 'const'
if self.specialtype == "nsid" and calltype == "in":
const = True

prefix = "*const " if const or shared else "*mut "
if "out" in calltype and self.isPtr(calltype):
prefix = "*mut " + prefix

if self.specialtype:
# The string types are very special, and need to be handled seperately.
if self.specialtype in ["cstring", "utf8string"]:
if calltype == "in":
return "*const ::nsstring::nsACString"
elif "out" in calltype:
return "*mut ::nsstring::nsACString"
else:
return "::nsstring::nsCString"
if self.specialtype == "astring":
if calltype == "in":
return "*const ::nsstring::nsAString"
elif "out" in calltype:
return "*mut ::nsstring::nsAString"
else:
return "::nsstring::nsString"
# nsid has some special handling, but generally re-uses the generic
# prefix handling above.
if self.specialtype == "nsid":
if "element" in calltype:
if self.isPtr(calltype):
raise IDLError(
"Array<nsIDPtr> not yet supported. "
"File an XPConnect bug if you need it.",
self.location,
)
return self.nativename
return prefix + self.nativename
raise RustNoncompat("special type %s unsupported" % self.specialtype)

# These 3 special types correspond to native pointer types which can
# generally be supported behind pointers. Other types are not supported
# for now.
if self.nativename == "void":
return prefix + "libc::c_void"
if self.nativename == "char":
return prefix + "libc::c_char"
if self.nativename == "char16_t":
return prefix + "u16"

if self.specialtype:
raise RustNoncompat("specialtype %s unsupported" % self.specialtype)
raise RustNoncompat("native type %s unsupported" % self.nativename)

def __str__(self):
Expand Down
4 changes: 2 additions & 2 deletions xpcom/rust/gtest/xpcom/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub unsafe extern "C" fn Rust_ObserveFromRust() -> *const interfaces::nsIObserve
&self,
_subject: *const interfaces::nsISupports,
topic: *const c_char,
_data: *const i16,
_data: *const u16,
) -> nsresult {
*self.run = true;
assert!(CStr::from_ptr(topic).to_str() == Ok("test-rust-observe"));
Expand Down Expand Up @@ -118,7 +118,7 @@ pub unsafe extern "C" fn Rust_GetMultipleInterfaces(
&self,
_subject: *const interfaces::nsISupports,
_topic: *const c_char,
_data: *const i16,
_data: *const u16,
) -> nsresult {
NS_OK
}
Expand Down

0 comments on commit c1e984d

Please sign in to comment.