From 22c8e9f60d0c06eb3eca43c5617a9f99c5ced0cd Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Tue, 18 Apr 2023 17:35:21 +0200 Subject: [PATCH] Don't leak ParseErrorList FFI crutch type into Rust Just like 16ea4380c (redirection.rs: don't leak FFI type into Rust code, 2023-04-09). --- fish-rust/src/ast.rs | 24 +++++++++--------- fish-rust/src/parse_constants.rs | 43 ++++++++++++++++---------------- fish-rust/src/parse_tree.rs | 16 ++++++------ src/parse_constants.h | 6 ++--- 4 files changed, 45 insertions(+), 44 deletions(-) diff --git a/fish-rust/src/ast.rs b/fish-rust/src/ast.rs index d1a0e877d..1e42eeb2f 100644 --- a/fish-rust/src/ast.rs +++ b/fish-rust/src/ast.rs @@ -13,8 +13,8 @@ use crate::common::{unescape_string, UnescapeStringStyle}; use crate::flog::FLOG; use crate::parse_constants::{ token_type_user_presentable_description, ParseError, ParseErrorCode, ParseErrorList, - ParseKeyword, ParseTokenType, ParseTreeFlags, SourceRange, StatementDecoration, - INVALID_PIPELINE_CMD_ERR_MSG, PARSE_FLAG_ACCEPT_INCOMPLETE_TOKENS, + ParseErrorListFfi, ParseKeyword, ParseTokenType, ParseTreeFlags, SourceRange, + StatementDecoration, INVALID_PIPELINE_CMD_ERR_MSG, PARSE_FLAG_ACCEPT_INCOMPLETE_TOKENS, PARSE_FLAG_CONTINUE_AFTER_ERROR, PARSE_FLAG_INCLUDE_COMMENTS, PARSE_FLAG_LEAVE_UNTERMINATED, PARSE_FLAG_SHOW_EXTRA_SEMIS, SOURCE_OFFSET_INVALID, }; @@ -2593,7 +2593,7 @@ macro_rules! parse_error_range { err.code = $code; err.source_start = $range.start(); err.source_length = $range.length(); - errors.0.push(err); + errors.push(err); } } } @@ -3912,7 +3912,7 @@ pub mod ast_ffi { type ParseTokenType = crate::parse_constants::ParseTokenType; type ParseKeyword = crate::parse_constants::ParseKeyword; type SourceRange = crate::parse_constants::SourceRange; - type ParseErrorList = crate::parse_constants::ParseErrorList; + type ParseErrorListFfi = crate::parse_constants::ParseErrorListFfi; type StatementDecoration = crate::parse_constants::StatementDecoration; } @@ -3971,12 +3971,12 @@ pub mod ast_ffi { unsafe fn ast_parse_ffi( src: &CxxWString, flags: u8, - errors: *mut ParseErrorList, + errors: *mut ParseErrorListFfi, ) -> Box; unsafe fn ast_parse_argument_list_ffi( src: &CxxWString, flags: u8, - errors: *mut ParseErrorList, + errors: *mut ParseErrorListFfi, ) -> Box; unsafe fn errored(self: &Ast) -> bool; #[cxx_name = "top"] @@ -4390,11 +4390,11 @@ impl Ast { } } -fn ast_parse_ffi(src: &CxxWString, flags: u8, errors: *mut ParseErrorList) -> Box { +fn ast_parse_ffi(src: &CxxWString, flags: u8, errors: *mut ParseErrorListFfi) -> Box { let mut out_errors: Option = if errors.is_null() { None } else { - Some(unsafe { &*errors }.clone()) + Some(unsafe { &(*errors).0 }.clone()) }; let ast = Box::new(Ast::parse( src.as_wstr(), @@ -4402,7 +4402,7 @@ fn ast_parse_ffi(src: &CxxWString, flags: u8, errors: *mut ParseErrorList) -> Bo &mut out_errors, )); if let Some(out_errors) = out_errors { - unsafe { *errors = out_errors }; + unsafe { (*errors).0 = out_errors }; } ast } @@ -4410,12 +4410,12 @@ fn ast_parse_ffi(src: &CxxWString, flags: u8, errors: *mut ParseErrorList) -> Bo fn ast_parse_argument_list_ffi( src: &CxxWString, flags: u8, - errors: *mut ParseErrorList, + errors: *mut ParseErrorListFfi, ) -> Box { let mut out_errors: Option = if errors.is_null() { None } else { - Some(unsafe { &*errors }.clone()) + Some(unsafe { &(*errors).0 }.clone()) }; let ast = Box::new(Ast::parse_argument_list( src.as_wstr(), @@ -4423,7 +4423,7 @@ fn ast_parse_argument_list_ffi( &mut out_errors, )); if let Some(out_errors) = out_errors { - unsafe { *errors = out_errors }; + unsafe { (*errors).0 = out_errors }; } ast } diff --git a/fish-rust/src/parse_constants.rs b/fish-rust/src/parse_constants.rs index 79f4531f8..beab14c84 100644 --- a/fish-rust/src/parse_constants.rs +++ b/fish-rust/src/parse_constants.rs @@ -213,17 +213,17 @@ mod parse_constants_ffi { skip_caret: bool, ) -> UniquePtr; - type ParseErrorList; - fn new_parse_error_list() -> Box; + type ParseErrorListFfi; + fn new_parse_error_list() -> Box; #[cxx_name = "offset_source_start"] - fn offset_source_start_ffi(self: &mut ParseErrorList, amt: usize); - fn size(self: &ParseErrorList) -> usize; - fn at(self: &ParseErrorList, offset: usize) -> *const ParseError; - fn empty(self: &ParseErrorList) -> bool; - fn push_back(self: &mut ParseErrorList, error: &parse_error_t); - fn append(self: &mut ParseErrorList, other: *mut ParseErrorList); - fn erase(self: &mut ParseErrorList, index: usize); - fn clear(self: &mut ParseErrorList); + fn offset_source_start_ffi(self: &mut ParseErrorListFfi, amt: usize); + fn size(self: &ParseErrorListFfi) -> usize; + fn at(self: &ParseErrorListFfi, offset: usize) -> *const ParseError; + fn empty(self: &ParseErrorListFfi) -> bool; + fn push_back(self: &mut ParseErrorListFfi, error: &parse_error_t); + fn append(self: &mut ParseErrorListFfi, other: *mut ParseErrorListFfi); + fn erase(self: &mut ParseErrorListFfi, index: usize); + fn clear(self: &mut ParseErrorListFfi); } extern "Rust" { @@ -632,12 +632,13 @@ fn token_type_user_presentable_description_ffi( token_type_user_presentable_description(type_, keyword).to_ffi() } -/// TODO This should be type alias once we drop the FFI. -#[derive(Clone)] -pub struct ParseErrorList(pub Vec); +pub type ParseErrorList = Vec; -unsafe impl ExternType for ParseErrorList { - type Id = type_id!("ParseErrorList"); +#[derive(Clone)] +pub struct ParseErrorListFfi(pub ParseErrorList); + +unsafe impl ExternType for ParseErrorListFfi { + type Id = type_id!("ParseErrorListFfi"); type Kind = cxx::kind::Opaque; } @@ -645,7 +646,7 @@ unsafe impl ExternType for ParseErrorList { /// errors in a substring of a larger source buffer. pub fn parse_error_offset_source_start(errors: &mut ParseErrorList, amt: usize) { if amt > 0 { - for ref mut error in errors.0.iter_mut() { + for ref mut error in errors.iter_mut() { // Preserve the special meaning of -1 as 'unknown'. if error.source_start != SOURCE_LOCATION_UNKNOWN { error.source_start += amt; @@ -654,13 +655,13 @@ pub fn parse_error_offset_source_start(errors: &mut ParseErrorList, amt: usize) } } -fn new_parse_error_list() -> Box { - Box::new(ParseErrorList(Vec::new())) +fn new_parse_error_list() -> Box { + Box::new(ParseErrorListFfi(Vec::new())) } -impl ParseErrorList { +impl ParseErrorListFfi { fn offset_source_start_ffi(&mut self, amt: usize) { - parse_error_offset_source_start(self, amt) + parse_error_offset_source_start(&mut self.0, amt) } fn size(&self) -> usize { @@ -679,7 +680,7 @@ impl ParseErrorList { self.0.push(error.into()) } - fn append(&mut self, other: *mut ParseErrorList) { + fn append(&mut self, other: *mut ParseErrorListFfi) { self.0.append(&mut (unsafe { &*other }.0.clone())); } diff --git a/fish-rust/src/parse_tree.rs b/fish-rust/src/parse_tree.rs index e48515218..c8aa624a2 100644 --- a/fish-rust/src/parse_tree.rs +++ b/fish-rust/src/parse_tree.rs @@ -5,9 +5,9 @@ use std::rc::Rc; use crate::ast::Ast; use crate::parse_constants::{ - token_type_user_presentable_description, ParseErrorCode, ParseErrorList, ParseKeyword, - ParseTokenType, ParseTreeFlags, SourceOffset, SourceRange, PARSE_FLAG_CONTINUE_AFTER_ERROR, - SOURCE_OFFSET_INVALID, + token_type_user_presentable_description, ParseErrorCode, ParseErrorList, ParseErrorListFfi, + ParseKeyword, ParseTokenType, ParseTreeFlags, SourceOffset, SourceRange, + PARSE_FLAG_CONTINUE_AFTER_ERROR, SOURCE_OFFSET_INVALID, }; use crate::tokenizer::TokenizerError; use crate::wchar::{wstr, WString, L}; @@ -137,7 +137,7 @@ mod parse_tree_ffi { extern "C++" { include!("ast.h"); pub type Ast = crate::ast::Ast; - pub type ParseErrorList = crate::parse_constants::ParseErrorList; + pub type ParseErrorListFfi = crate::parse_constants::ParseErrorListFfi; } extern "Rust" { type ParsedSourceRefFFI; @@ -148,7 +148,7 @@ mod parse_tree_ffi { fn parse_source_ffi( src: &CxxWString, flags: u8, - errors: *mut ParseErrorList, + errors: *mut ParseErrorListFfi, ) -> Box; fn clone(self: &ParsedSourceRefFFI) -> Box; fn src(self: &ParsedSourceRefFFI) -> &CxxWString; @@ -175,16 +175,16 @@ fn new_parsed_source_ref(src: &CxxWString, ast: Pin<&mut Ast>) -> Box Box { let mut out_errors: Option = if errors.is_null() { None } else { - Some(unsafe { &*errors }.clone()) + Some(unsafe { &(*errors).0 }.clone()) }; let ps = parse_source(src.from_ffi(), ParseTreeFlags(flags), &mut out_errors); if let Some(out_errors) = out_errors { - unsafe { *errors = out_errors }; + unsafe { (*errors).0 = out_errors }; } Box::new(ParsedSourceRefFFI(ps)) diff --git a/src/parse_constants.h b/src/parse_constants.h index 41e8fd4d5..6fdf83a27 100644 --- a/src/parse_constants.h +++ b/src/parse_constants.h @@ -23,7 +23,7 @@ using parse_keyword_t = ParseKeyword; using statement_decoration_t = StatementDecoration; using parse_error_code_t = ParseErrorCode; using pipeline_position_t = PipelinePosition; -using parse_error_list_t = ParseErrorList; +using parse_error_list_t = ParseErrorListFfi; #else @@ -101,8 +101,8 @@ enum class parse_error_code_t : uint8_t { andor_in_pipeline, }; -struct ParseErrorList; -using parse_error_list_t = ParseErrorList; +struct ParseErrorListFfi; +using parse_error_list_t = ParseErrorListFfi; #endif