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

Port unibyte-char-to-multibyte function #218

Merged
merged 5 commits into from Jul 14, 2017

Conversation

Projects
None yet
3 participants
@shanavas786
Collaborator

shanavas786 commented Jul 9, 2017

No description provided.

let mut c = ch.as_fixnum().unwrap() as u32;
if c >= 0x100 {
unsafe {
error("Not a unibyte character: %d".as_ptr(), c);

This comment has been minimized.

@DavidDeSimone

DavidDeSimone Jul 9, 2017

Collaborator

I believe this should be error("Not a unibyte character, %d\0".as_ptr(), c), as Rust does not null-terminate it's string literals automatically.

@DavidDeSimone

This comment has been minimized.

Collaborator

DavidDeSimone commented Jul 9, 2017

Looks like the build is failing due to the release rustc 1.20.0-nightly not playing well with alloc_unexecmacosx. I'll take a look at that this morning if no one else does.

Other then my one inline comment, this looks like a solid PR 👍

@shanavas786

This comment has been minimized.

Collaborator

shanavas786 commented Jul 10, 2017

@DavidDeSimone Thanks, updated

/// Check if Lisp object is a character or not.
/// Similar to CHECK_CHARACTER
#[inline]
pub fn is_character_or_error(self) -> () {

This comment has been minimized.

@birkenfeld

birkenfeld Jul 10, 2017

Collaborator

Please make this as_character_or_error. Then you can make the cast to fixnum and ultimately to Codepoint here already.

/// UNIBYTE_TO_CHAR macro
#[inline]
pub fn unibyte_to_char(cp: Codepoint) -> Codepoint {
if (cp as u8).is_ascii() {

This comment has been minimized.

@birkenfeld

birkenfeld Jul 10, 2017

Collaborator

This cast is dangerous - it will truncate the codepoint so that thinks that aren't ascii will look like ascii.

#[inline]
#[allow(unused_comparisons)]
pub fn make_char_multibyte(cp: Codepoint) -> Codepoint {
debug_assert!((cp) >= 0 && (cp) < 256);

This comment has been minimized.

@birkenfeld

birkenfeld Jul 10, 2017

Collaborator

You can just omit the >= 0 comparison, and remove the allow.

// Same as CHECK_TYPE macro,
// order of arguments changed
#[inline]
#[allow(dead_code)]

This comment has been minimized.

@birkenfeld

birkenfeld Jul 10, 2017

Collaborator

Why is it dead code?

#[lisp_fn]
fn unibyte_char_to_multibyte(ch: LispObject) -> LispObject {
ch.is_character_or_error();
let mut c = ch.as_fixnum().unwrap() as u32;

This comment has been minimized.

@birkenfeld

birkenfeld Jul 10, 2017

Collaborator

no need for mut - just do either direct return or let below.

@shanavas786 shanavas786 force-pushed the shanavas786:char-funcs branch from 02d236e to 29ed35a Jul 10, 2017

@@ -30,13 +30,11 @@ fn char_or_string_p(object: LispObject) -> LispObject {
/// Convert the byte CH to multibyte character.
#[lisp_fn]
fn unibyte_char_to_multibyte(ch: LispObject) -> LispObject {
ch.is_character_or_error();
let mut c = ch.as_fixnum().unwrap() as u32;
let c = ch.as_character_or_error() as u32;

This comment has been minimized.

@birkenfeld

birkenfeld Jul 10, 2017

Collaborator

The "as u32" should be unnecessary (especially since Codepoint is just an alias for it 😄)

This comment has been minimized.

@shanavas786

shanavas786 Jul 10, 2017

Collaborator

yes it is, updated :)

@shanavas786 shanavas786 force-pushed the shanavas786:char-funcs branch 2 times, most recently from f8cac5a to 7f5c3d0 Jul 10, 2017

@shanavas786

This comment has been minimized.

Collaborator

shanavas786 commented Jul 12, 2017

Conflicts resolved

@DavidDeSimone

This comment has been minimized.

Collaborator

DavidDeSimone commented Jul 12, 2017

@shanavas786 OSX build fix is on master. If you merge in master travis should go green.

@shanavas786 shanavas786 force-pushed the shanavas786:char-funcs branch from 7f5c3d0 to 2a53970 Jul 13, 2017

@shanavas786

This comment has been minimized.

Collaborator

shanavas786 commented Jul 13, 2017

@DavidDeSimone done :)

@birkenfeld birkenfeld merged commit 295e1f6 into Wilfred:master Jul 14, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@birkenfeld

This comment has been minimized.

Collaborator

birkenfeld commented Jul 14, 2017

Thanks!

@shanavas786 shanavas786 deleted the shanavas786:char-funcs branch Jul 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment