Skip to content
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

Add CString implementation #372

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub use class::Class;
pub use persistent::{Outlive, Persistent};
pub use result::{CatchResultExt, CaughtError, CaughtResult, Error, Result, ThrowResultExt};
pub use value::{
array, atom, convert, function, module, object, promise, Array, Atom, BigInt, Coerced,
array, atom, convert, function, module, object, promise, Array, Atom, BigInt, CString, Coerced,
Exception, Filter, FromAtom, FromIteratorJs, FromJs, Function, IntoAtom, IntoJs, IteratorJs,
Module, Null, Object, Promise, String, Symbol, Type, Undefined, Value,
};
Expand Down
5 changes: 3 additions & 2 deletions core/src/persistent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use crate::{
atom::{self, Atom},
qjs,
value::Constructor,
Array, BigInt, Ctx, Error, Exception, FromJs, Function, IntoJs, Module, Object, Promise,
Result, String, Symbol, Value,
Array, BigInt, CString, Ctx, Error, Exception, FromJs, Function, IntoJs, Module, Object,
Promise, Result, String, Symbol, Value,
};

use std::{
Expand Down Expand Up @@ -56,6 +56,7 @@ outlive_impls! {
Value,
Symbol,
String,
CString,
Object,
Array,
BigInt,
Expand Down
2 changes: 1 addition & 1 deletion core/src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub use function::{Constructor, Function};
pub use module::Module;
pub use object::{Filter, Object};
pub use promise::Promise;
pub use string::String;
pub use string::{CString, String};
pub use symbol::Symbol;

#[cfg(feature = "array-buffer")]
Expand Down
10 changes: 8 additions & 2 deletions core/src/value/convert/from.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{
convert::List, Array, Ctx, Error, FromAtom, FromJs, Object, Result, StdString, String, Type,
Value,
convert::List, Array, CString, Ctx, Error, FromAtom, FromJs, Object, Result, StdString, String,
Type, Value,
};
use std::{
cell::{Cell, RefCell},
Expand Down Expand Up @@ -48,6 +48,12 @@ impl<'js> FromJs<'js> for char {
}
}

impl<'js> FromJs<'js> for CString<'js> {
fn from_js(_ctx: &Ctx<'js>, value: Value<'js>) -> Result<Self> {
String::from_value(value)?.to_cstring()
}
}

/// Convert from JS as any
impl<'js> FromJs<'js> for () {
fn from_js(_: &Ctx<'js>, _: Value<'js>) -> Result<Self> {
Expand Down
15 changes: 14 additions & 1 deletion core/src/value/convert/into.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::{
convert::{IteratorJs, List},
value::Constructor,
Array, Ctx, Error, IntoAtom, IntoJs, Object, Result, StdResult, StdString, String, Value,
Array, CString, Ctx, Error, IntoAtom, IntoJs, Object, Result, StdResult, StdString, String,
Value,
};
use std::{
cell::{Cell, RefCell},
Expand Down Expand Up @@ -52,6 +53,18 @@ impl<'js> IntoJs<'js> for char {
}
}

impl<'js> IntoJs<'js> for CString<'js> {
fn into_js(self, ctx: &Ctx<'js>) -> Result<Value<'js>> {
String::from_str(ctx.clone(), self.as_str()).map(|String(value)| value)
}
}

impl<'js> IntoJs<'js> for &CString<'js> {
fn into_js(self, ctx: &Ctx<'js>) -> Result<Value<'js>> {
String::from_str(ctx.clone(), self.as_str()).map(|String(value)| value)
}
}

impl<'js, T> IntoJs<'js> for &[T]
where
for<'a> &'a T: IntoJs<'js>,
Expand Down
88 changes: 87 additions & 1 deletion core/src/value/string.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{qjs, Ctx, Error, Result, StdString, Value};
use std::{mem, slice, str};
use std::{ffi::c_char, mem, slice, str};

/// Rust representation of a JavaScript string.
#[derive(Debug, Clone, PartialEq, Hash)]
Expand All @@ -25,6 +25,11 @@ impl<'js> String<'js> {
Ok(result?)
}

/// Convert the Javascript string to a Javascript C string.
pub fn to_cstring(self) -> Result<CString<'js>> {
CString::from_string(self)
}

/// Create a new JavaScript string from an Rust string.
pub fn from_str(ctx: Ctx<'js>, s: &str) -> Result<Self> {
let len = s.as_bytes().len();
Expand All @@ -37,6 +42,66 @@ impl<'js> String<'js> {
}
}

/// Rust representation of a JavaScript C string.
#[derive(Debug)]
pub struct CString<'js> {
ptr: *const c_char,
Copy link
Owner

Choose a reason for hiding this comment

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

This should probably be a NonNull.
Feels more correct as this pointer should never be null and might allow for more optimizations within enums.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed

len: usize,
ctx: Ctx<'js>,
}

#[allow(dead_code)]
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this attribute here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bad copy paste

impl<'js> CString<'js> {
/// Create a new JavaScript C string from a JavaScript string.
pub fn from_string(string: String<'js>) -> Result<Self> {
let mut len = mem::MaybeUninit::uninit();
// SAFETY: The pointer points to a JSString content which is ref counted
let ptr = unsafe {
qjs::JS_ToCStringLen(string.0.ctx.as_ptr(), len.as_mut_ptr(), string.as_raw())
};
if ptr.is_null() {
// Might not ever happen but I am not 100% sure
// so just incase check it.
return Err(Error::Unknown);
}
let len = unsafe { len.assume_init() };
Ok(Self {
ptr,
len,
ctx: string.0.ctx.clone(),
})
}

/// Converts a `CString` to a raw pointer.
pub fn as_ptr(&self) -> *const c_char {
self.ptr
}

/// Returns the length of this `CString`, in bytes (not chars or graphemes).
pub fn len(&self) -> usize {
self.len
}

/// Returns `true` if this `CString` has a length of zero, and `false` otherwise.
pub fn is_empty(&self) -> bool {
self.len == 0
}

/// Extracts a string slice containing the entire `CString`.
pub fn as_str(&self) -> &str {
// SAFETY: The pointer points to a JSString content which is ref counted
let bytes = unsafe { slice::from_raw_parts(self.ptr as *const u8, self.len) };
// SAFETY: The bytes are garanteed to be valid utf8 by QuickJS
unsafe { str::from_utf8_unchecked(bytes) }
}
}

impl<'js> Drop for CString<'js> {
fn drop(&mut self) {
unsafe { qjs::JS_FreeCString(self.ctx.as_ptr(), self.ptr) };
}
}

#[cfg(test)]
mod test {
use crate::{prelude::*, *};
Expand All @@ -57,4 +122,25 @@ mod test {
assert_eq!(text, "foobar".to_string());
});
}

#[test]
fn from_javascript_c() {
test_with(|ctx| {
let s: CString = ctx.eval(" 'foo bar baz' ").unwrap();
assert_eq!(s.as_str(), "foo bar baz");
});
}

#[test]
fn to_javascript_c() {
test_with(|ctx| {
let string = String::from_str(ctx.clone(), "foo")
.unwrap()
.to_cstring()
.unwrap();
let func: Function = ctx.eval("x => x + 'bar'").unwrap();
let text: StdString = (string,).apply(&func).unwrap();
assert_eq!(text, "foobar".to_string());
});
}
}