From 53fae97ca082a2ec7468017989fc122fcbfdf1aa Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Tue, 7 May 2019 17:24:38 -0400 Subject: [PATCH] Migrate playground-utils to ffi_support This is a test run of using the patterns encouraged by mozilla's ffi-support crate (https://docs.rs/ffi-support). I wasn't totally sure how this would go, but it was relatively painless and I'm encouraged by the code. While I was in here I also did some general cleanup on the swift side --- .../RustPlayground/Playground.swift | 77 +++++++------------ playground-utils-ffi/Cargo.toml | 1 + playground-utils-ffi/playground.h | 9 ++- playground-utils-ffi/src/lib.rs | 62 ++++++--------- playground-utils/Cargo.toml | 3 +- playground-utils/src/error.rs | 11 ++- 6 files changed, 68 insertions(+), 95 deletions(-) diff --git a/RustPlayground/RustPlayground/Playground.swift b/RustPlayground/RustPlayground/Playground.swift index 83234c4..833f870 100644 --- a/RustPlayground/RustPlayground/Playground.swift +++ b/RustPlayground/RustPlayground/Playground.swift @@ -12,46 +12,25 @@ import Foundation struct PlaygroundError { let message: String - let code: Int - - init?(json: [String: AnyObject]) { - if let message = json["message"] as? String, - let code = json["code"] as? Int { - self.message = message - self.code = code - } else { - return nil - } - } + let code: Int32 } extension PlaygroundError: Error {} func listToolchains() -> Result<[Toolchain], PlaygroundError> { - let response = JsonResponse.from(externFn: playgroundGetToolchains) - switch response { - case .ok(let result): - let data = (result as! [AnyObject]) - let toolchains = data.map { Toolchain.fromJson(json: $0)! } - return .success(toolchains) - case .error(let error): - return .failure(error) - } + let response = PlaygroundResult.from { err in playgroundGetToolchains(err) } + return response.map({ (toolchains) -> [Toolchain] in + let data = toolchains as! [AnyObject] + return data.map { Toolchain.fromJson(json: $0)! } + }) } func executeTask(inDirectory buildDir: URL, task: CompilerTask, stderr: @escaping ((String) -> ())) -> Result { GlobalTaskContext.stderrCallback = stderr let buildPath = buildDir.path let taskJson = task.toJson() - let response = JsonResponse.from { playgroundExecuteTask(buildPath, taskJson, stderrCallback) } - - switch response { - case .ok(let result): - let data = result as! [String: AnyObject] - return .success(CompilerResult.fromJson(data)) - case .error(let err): - return .failure(err) - } + let response = PlaygroundResult.from { err in playgroundExecuteTask(buildPath, taskJson, stderrCallback, err) } + return response.map { CompilerResult.fromJson($0 as! [String: AnyObject]) } } /// The least bad way I could think of to pipe callbacks from rust back @@ -96,30 +75,26 @@ struct Toolchain { return base } } -// TODO: this can probably just be a Result -enum JsonResponse { - case error(PlaygroundError) - case ok(Any) - - /// Wrapper around an external function that returns json. Handles basic - /// parsing and freeing memory. - /// - /// we control all the messages we send and receive, so we assume all - /// messages are well-formed. - static func from(externFn: () -> UnsafePointer?) -> JsonResponse { - let cString = externFn()! + +typealias PlaygroundResult = Result + +extension PlaygroundResult { + /// Call an external function, doing error handling and json parsing. + static func from(externFn: (UnsafeMutablePointer) -> UnsafePointer?) -> PlaygroundResult { + var error = ExternError() + let cString = externFn(&error)! defer { playgroundStringFree(cString) } - let string = String(cString: cString, encoding: .utf8)! - let message = try! JSONSerialization.jsonObject(with: string.data(using: .utf8)!) as! [String: AnyObject] - if let result = message["result"] { - return .ok(result) - } else if let error = message["error"] { - let error = error as! [String: AnyObject] - return .error(PlaygroundError(json: error)!) - } else { - fatalError("invalid json response: \(message)") + if error.code != 0 { + let message = String(cString: error.message, encoding: .utf8)! + let error = PlaygroundError(message: message, code: error.code) + playgroundStringFree(error.message) + return .failure(error) } + + let string = String(cString: cString, encoding: .utf8)! + let message = try! JSONSerialization.jsonObject(with: string.data(using: .utf8)!) + return .success(message) } -} +} diff --git a/playground-utils-ffi/Cargo.toml b/playground-utils-ffi/Cargo.toml index 06fedb4..c405f7b 100644 --- a/playground-utils-ffi/Cargo.toml +++ b/playground-utils-ffi/Cargo.toml @@ -5,6 +5,7 @@ authors = ["Colin Rofls "] edition = "2018" [dependencies] +ffi-support = "0.3.4" serde_json = "1.0" [dependencies.playground-utils] diff --git a/playground-utils-ffi/playground.h b/playground-utils-ffi/playground.h index c37bf09..2a24d66 100644 --- a/playground-utils-ffi/playground.h +++ b/playground-utils-ffi/playground.h @@ -4,8 +4,13 @@ typedef const char* json; typedef void (*stderr_callback)(const char*); -extern json playgroundGetToolchains(); -extern json playgroundExecuteTask(const char* path, json, stderr_callback); +typedef struct _ExternError { + int32_t code; + char *message; // note: nullable +} ExternError; + +extern json playgroundGetToolchains(ExternError* error); +extern json playgroundExecuteTask(const char* path, json, stderr_callback, ExternError* error); extern void playgroundStringFree(json); diff --git a/playground-utils-ffi/src/lib.rs b/playground-utils-ffi/src/lib.rs index bdd2cb3..f7e7a7a 100644 --- a/playground-utils-ffi/src/lib.rs +++ b/playground-utils-ffi/src/lib.rs @@ -1,23 +1,17 @@ -#[macro_use] -extern crate serde_json; - use libc::c_char; use std::ffi::{CStr, CString, OsStr}; use std::os::unix::ffi::OsStrExt; use std::path::Path; -use playground_utils::{do_compile_task, list_toolchains, Error, Task}; +use ffi_support::{call_with_result, ExternError}; +use playground_utils::{do_compile_task, list_toolchains, Task}; #[no_mangle] -pub extern "C" fn playgroundGetToolchains() -> *const c_char { - let response = match list_toolchains() { - Ok(toolchains) => json!({ "result": toolchains }), - Err(e) => to_json_error(e), - }; - - let response_str = serde_json::to_string(&response).unwrap(); - let cstring = CString::new(response_str).expect("nul byte in response json"); - cstring.into_raw() +pub extern "C" fn playgroundGetToolchains(err: &mut ExternError) -> *const c_char { + call_with_result(err, || { + list_toolchains() + .map(|r| serde_json::to_string(&r).unwrap()) + }) } #[no_mangle] @@ -25,25 +19,22 @@ pub extern "C" fn playgroundExecuteTask( path: *const c_char, cmd_json: *const c_char, std_err_callback: extern "C" fn(*const c_char), + err: &mut ExternError, ) -> *const c_char { - eprintln!("playground execute task"); - let path = unsafe { CStr::from_ptr(path) }; - let json = unsafe { CStr::from_ptr(cmd_json) }; - let path = Path::new(OsStr::from_bytes(path.to_bytes())); - let json = json.to_str().expect("json must be valid utf8"); - let task: Task = serde_json::from_str(json).expect("malformed task json"); - let response = match do_compile_task(path, task, |stderr| { - let cstring = - CString::new(stderr).unwrap_or_else(|_| CString::new("null byte in stderr").unwrap()); - std_err_callback(cstring.as_ptr()); - }) { - Ok(result) => json!({ "result": result }), - Err(e) => to_json_error(e), - }; - - let response_str = serde_json::to_string(&response).unwrap(); - let cstring = CString::new(response_str).expect("nul byte in response json"); - cstring.into_raw() + call_with_result(err, || { + eprintln!("playground execute task"); + let path = unsafe { CStr::from_ptr(path) }; + let json = unsafe { CStr::from_ptr(cmd_json) }; + let path = Path::new(OsStr::from_bytes(path.to_bytes())); + let json = json.to_str().expect("json must be valid utf8"); + let task: Task = serde_json::from_str(json).expect("malformed task json"); + do_compile_task(path, task, |stderr| { + let cstring = CString::new(stderr) + .unwrap_or_else(|_| CString::new("null byte in stderr").unwrap()); + std_err_callback(cstring.as_ptr()); + }) + .map(|r| serde_json::to_string(&r).unwrap()) + }) } #[no_mangle] @@ -56,12 +47,3 @@ pub extern "C" fn playgroundStringFree(ptr: *mut c_char) { CString::from_raw(ptr); } } - -fn to_json_error(e: Error) -> serde_json::Value { - json!({ - "error": { - "message": e.to_string(), - "code": e.error_code(), - } - }) -} diff --git a/playground-utils/Cargo.toml b/playground-utils/Cargo.toml index d136e3f..3eb16d7 100644 --- a/playground-utils/Cargo.toml +++ b/playground-utils/Cargo.toml @@ -1,11 +1,12 @@ [package] name = "playground-utils" -version = "0.1.0" +version = "0.2.0" authors = ["Colin Rofls "] edition = "2018" [dependencies] dirs = "1.0" +ffi-support = "0.3.4" serde = "1.0" serde_derive = "1.0" semver = "0.9" diff --git a/playground-utils/src/error.rs b/playground-utils/src/error.rs index f82bd9f..b5ce363 100644 --- a/playground-utils/src/error.rs +++ b/playground-utils/src/error.rs @@ -3,6 +3,8 @@ use std::io; use std::path::PathBuf; use std::process::Output; +use ffi_support::{ExternError, ErrorCode}; + #[derive(Debug)] pub enum Error { ToolchainParseError(String), @@ -36,7 +38,7 @@ impl Error { /// An error code included in the json if this is sent to core. /// This is not systematic. We just want to be able to easily identify /// certain cases, such as when Rustup is not installed. - pub fn error_code(&self) -> u32 { + pub fn error_code(&self) -> i32 { use Error::*; match self { BadExit(_) => 1, @@ -71,3 +73,10 @@ impl fmt::Display for Error { } impl std::error::Error for Error {} + +impl From for ExternError { + fn from(e: Error) -> ExternError { + let code = ErrorCode::new(e.error_code()); + ExternError::new_error(code, e.to_string()) + } +}