diff --git a/Cargo.lock b/Cargo.lock index b2d119f..de7a6ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -226,6 +226,12 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "dissimilar" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "86e3bdc80eee6e16b2b6b0f87fbc98c04bee3455e35174c0de1a125d0688c632" + [[package]] name = "either" version = "1.9.0" @@ -315,6 +321,16 @@ version = "0.2.2" name = "example-tutorial" version = "0.2.2" +[[package]] +name = "expect-test" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "30d9eafeadd538e68fb28016364c9732d78e420b9ff8853fa5e4058861e9f8d3" +dependencies = [ + "dissimilar", + "once_cell", +] + [[package]] name = "fd-lock" version = "3.0.13" @@ -576,6 +592,15 @@ version = "1.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9e08d8363704e6c71fc928674353e6b7c23dcea9d82d7012c8faf2a3a025f8d0" +[[package]] +name = "strip-ansi-escapes" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "55ff8ef943b384c414f54aefa961dd2bd853add74ec75e7ac74cf91dba62bcfa" +dependencies = [ + "vte", +] + [[package]] name = "strsim" version = "0.10.0" @@ -623,6 +648,26 @@ version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" +[[package]] +name = "vte" +version = "0.11.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f5022b5fbf9407086c180e9557be968742d839e68346af7792b8592489732197" +dependencies = [ + "utf8parse", + "vte_generate_state_changes", +] + +[[package]] +name = "vte_generate_state_changes" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d257817081c7dffcdbab24b9e62d2def62e2ff7d00b1c20062551e6cccc145ff" +dependencies = [ + "proc-macro2", + "quote", +] + [[package]] name = "winapi" version = "0.3.9" @@ -780,6 +825,8 @@ version = "0.2.2" dependencies = [ "ariadne", "chumsky", + "expect-test", "iter_tools", + "strip-ansi-escapes", "zngur-def", ] diff --git a/book/src/call_cpp_from_rust/index.md b/book/src/call_cpp_from_rust/index.md index 31a7f6b..639c2ed 100644 --- a/book/src/call_cpp_from_rust/index.md +++ b/book/src/call_cpp_from_rust/index.md @@ -6,7 +6,7 @@ most of the existing code is in C++ and Rust code needs a way to use them in ord direction. Zngur general idea is that Rust semantics is a subset of C++ semantics, so we should use Rust things in C++ and avoid bringing -C++ things in Rust (See [Philosophy](../philosophy.md)). So, even in the C++ to Rust direction, Zngur operates only on Rust types. For +C++ things in Rust (See [Design decisions](../philosophy.md)). So, even in the C++ to Rust direction, Zngur operates only on Rust types. For example, Zngur allows you to call a C++ function that takes Rust types in inputs in Rust, but you can't call a function that takes a C++ object. Or you can write an `impl` block for a Rust type in C++ and call those methods in Rust, but you can't call C++ methods of a C++ object in Rust. diff --git a/book/src/call_cpp_from_rust/opaque.md b/book/src/call_cpp_from_rust/opaque.md index 0866b31..cae193b 100644 --- a/book/src/call_cpp_from_rust/opaque.md +++ b/book/src/call_cpp_from_rust/opaque.md @@ -2,6 +2,56 @@ ## Opaque borrowed C++ type +The generated Rust code contains a `ZngurCppOpaqueBorrowedObject` which you can use to create opaque Rust equivalent for C++ types. To do +that, you first need to create a newtype wrapper around it in Rust: + +```Rust +struct Way(generated::ZngurCppOpaqueBorrowedObject); +``` + +Then you need to add this type to your `main.zng` as a `#cpp_ref`: + +``` +type crate::Way { + #cpp_ref "::osmium::Way"; +} +``` + +Note that `#cpp_ref` types don't need manual layout policy. This enables creating `rust::Ref` from a `const osmium::Way&` in C++ and +you can pass it to the Rust side. Rust side can't do anything meaningful with it, except passing it again to the C++ side. In the C++ +side `rust::Ref` has a `.cpp()` method which will return the `osmium::Way&` back to you. If you want to use the methods on +your C++ type in the Rust side, you can write `impl` and `impl trait` blocks for the newtype wrapper `crate::Way` inside C++. See +the [`examples/osmium`](https://github.com/HKalbasi/zngur/blob/main/examples/osmium) for a full working example. + ## Opaque owned C++ type -Owning C++ type in the Rust stack is impossible without a huge amount of acrobatics. +Owning C++ type in the Rust stack is impossible without a huge amount of acrobatics, since Rust assumes that every type is memcpy-movable, which doesn't +work for C++ types with non trivial move constructors. It is not entirely impossible, for example the `moveit` crate achieves it by hiding the binding +of the stack owner in a macro: + +```Rust +moveit! { + let mut stack_obj = ffi::A::new(); +} +// here, type of `stack_obj` is `Pin>`, not `ffi::A` itself. +stack_obj.as_mut().set(42); +assert_eq!(stack_obj.get(), 42); +``` + +Keeping the Rust side clean and idiomatic is one of the design goals of Zngur, and such a macro is not clean and idiomatic Rust. So storing C++ +objects in the Rust stack is not supported. If you really need to store things in the Rust stack, consider moving the type definition into Rust. + +Keeping C++ object in Rust using heap allocation is supported with `ZngurCppOpaqueOwnedObject`. + +## Semantics of the opaque types + +The `ZngurCppOpaqueBorrowedObject` and newtype wrappers around it don't represent a C++ object, but they represent an imaginary ZST Rust object at the first +byte of a C++ object. This can sometimes cause behavior that is safe and sound, but surprising and counterintuitive for someone that expects them +to represent the whole C++ object. Some examples (assume `RustType` is a newtype wrapper around `ZngurCppOpaqueBorrowedObject` that refers to a +`CppType` class in the C++): + +- `std::mem::sizeof::()` is 0, not the size of `CppType` +- `std::mem::alignof::()` is 1, not the align of `CppType` +- `std::mem::swap::(a, b)` only swaps the first zero bytes of those, i.e. does nothing. + +Those problem might be solved by the `extern type` language feature. diff --git a/examples/osmium/main.zng b/examples/osmium/main.zng index 3d0f423..343a460 100644 --- a/examples/osmium/main.zng +++ b/examples/osmium/main.zng @@ -43,26 +43,18 @@ type crate::Reader { } type crate::Way { - layout(size = 0, align = 1); - #cpp_ref "::osmium::Way"; } type crate::WayNodeList { - layout(size = 0, align = 1); - #cpp_ref "::osmium::WayNodeList"; } type crate::TagList { - layout(size = 0, align = 1); - #cpp_ref "::osmium::TagList"; } type crate::Node { - layout(size = 0, align = 1); - #cpp_ref "::osmium::NodeRef"; } diff --git a/zngur-generator/src/lib.rs b/zngur-generator/src/lib.rs index f41a096..400190e 100644 --- a/zngur-generator/src/lib.rs +++ b/zngur-generator/src/lib.rs @@ -25,8 +25,8 @@ pub use zngur_def::*; pub struct ZngurGenerator(ZngurFile); impl ZngurGenerator { - pub fn build_from_zng(zng: ParsedZngFile<'_>) -> Self { - ZngurGenerator(zng.into_zngur_file()) + pub fn build_from_zng(zng: ZngurFile) -> Self { + ZngurGenerator(zng) } pub fn render(self) -> (String, String, Option) { diff --git a/zngur-parser/Cargo.toml b/zngur-parser/Cargo.toml index bb12170..364bc18 100644 --- a/zngur-parser/Cargo.toml +++ b/zngur-parser/Cargo.toml @@ -12,3 +12,7 @@ ariadne = "0.3.0" chumsky = { version = "1.0.0-alpha.4", features = ["label"] } iter_tools = "0.1.4" zngur-def = { version = "=0.2.2", path = "../zngur-def" } + +[dev-dependencies] +expect-test = "1.4.1" +strip-ansi-escapes = "0.2.0" diff --git a/zngur-parser/src/lib.rs b/zngur-parser/src/lib.rs index 667dec4..e39d629 100644 --- a/zngur-parser/src/lib.rs +++ b/zngur-parser/src/lib.rs @@ -1,6 +1,6 @@ use std::{fmt::Display, process::exit, sync::Mutex}; -use ariadne::{sources, Color, Label, Report, ReportKind, Source}; +use ariadne::{sources, Color, Label, Report, ReportKind}; use chumsky::prelude::*; use iter_tools::{Either, Itertools}; @@ -12,6 +12,9 @@ use zngur_def::{ pub type Span = SimpleSpan; +#[cfg(test)] +mod tests; + #[derive(Debug, Clone, Copy, PartialEq, Eq)] struct Spanned { inner: T, @@ -66,7 +69,7 @@ enum ParsedItem<'a> { }, Type { ty: Spanned>, - items: Vec>, + items: Vec>>, }, Trait { tr: ParsedRustTrait<'a>, @@ -165,6 +168,8 @@ impl ParsedItem<'_> { let mut cpp_value = None; let mut cpp_ref = None; for item in items { + let item_span = item.span; + let item = item.inner; match item { ParsedTypeItem::Layout(span, p) => { layout = Some(match p { @@ -239,6 +244,16 @@ impl ParsedItem<'_> { cpp_value = Some((field.to_owned(), cpp_type.to_owned())); } ParsedTypeItem::CppRef { cpp_type } => { + match layout_span { + Some(span) => { + create_and_emit_error("Duplicate layout policy found", span); + } + None => { + layout = + Some(LayoutPolicy::StackAllocated { size: 0, align: 1 }); + layout_span = Some(item_span); + } + } cpp_ref = Some(cpp_type.to_owned()); } } @@ -260,18 +275,27 @@ impl ParsedItem<'_> { } if let Some(is_unsized) = is_unsized { if let Some(span) = layout_span { + let file_name = LATEST_FILENAME.lock().unwrap().to_owned(); emit_ariadne_error( - Report::build(ReportKind::Error, (), span.start) - .with_message("Duplicate layout policy found for unsized type.") - .with_label(Label::new(span.start..span.end).with_message( - "Unsized types have implicit layout policy, remove this.", - ).with_color(Color::Red)) - .with_label( - Label::new(is_unsized.span.start..is_unsized.span.end) - .with_message("Type declared as unsized here.") - .with_color(Color::Blue), - ) - .finish(), + Report::build( + ReportKind::Error, + file_name.clone(), + span.start, + ) + .with_message("Duplicate layout policy found for unsized type.") + .with_label( + Label::new((file_name.clone(), span.start..span.end)) + .with_message( + "Unsized types have implicit layout policy, remove this.", + ) + .with_color(Color::Red), + ) + .with_label( + Label::new((file_name.clone(), is_unsized.span.start..is_unsized.span.end)) + .with_message("Type declared as unsized here.") + .with_color(Color::Blue), + ) + .finish(), ) } layout = Some(LayoutPolicy::OnlyByRef); @@ -434,11 +458,7 @@ static LATEST_FILENAME: Mutex = Mutex::new(String::new()); static LATEST_TEXT: Mutex = Mutex::new(String::new()); impl ParsedZngFile<'_> { - pub fn parse( - filename: &str, - text: &str, - then: impl for<'a> Fn(ParsedZngFile<'a>) -> T, - ) -> T { + pub fn parse(filename: &str, text: &str) -> ZngurFile { *LATEST_FILENAME.lock().unwrap() = filename.to_string(); *LATEST_TEXT.lock().unwrap() = text.to_string(); let (tokens, errs) = lexer().parse(text).into_output_errors(); @@ -454,7 +474,7 @@ impl ParsedZngFile<'_> { let errs = errs.into_iter().map(|e| e.map_token(|c| c.to_string())); emit_error(errs); }; - then(ast.0) + ast.0.into_zngur_file() } pub fn into_zngur_file(self) -> ZngurFile { @@ -470,31 +490,49 @@ fn create_and_emit_error<'a>(error: &str, span: Span) -> ! { emit_error([Rich::custom(span, error)].into_iter()) } -fn emit_ariadne_error<'a>(err: Report<'_>) -> ! { - err.eprint(Source::from(&**LATEST_TEXT.lock().unwrap())) - .unwrap(); - exit(101); +#[cfg(test)] +fn emit_ariadne_error(err: Report<'_, (String, std::ops::Range)>) -> ! { + let mut r = Vec::::new(); + // Block needed to drop lock guards before panic + { + let filename = &**LATEST_FILENAME.lock().unwrap(); + let text = &**LATEST_TEXT.lock().unwrap(); + + err.write(sources([(filename.to_string(), text)]), &mut r) + .unwrap(); + } + std::panic::resume_unwind(Box::new(tests::ErrorText( + String::from_utf8(strip_ansi_escapes::strip(r)).unwrap(), + ))); } -fn emit_error<'a>(errs: impl Iterator>) -> ! { +#[cfg(not(test))] +fn emit_ariadne_error(err: Report<'_, (String, std::ops::Range)>) -> ! { let filename = &**LATEST_FILENAME.lock().unwrap(); let text = &**LATEST_TEXT.lock().unwrap(); + + err.eprint(sources([(filename.to_string(), text)])).unwrap(); + exit(101); +} + +fn emit_error<'a>(errs: impl Iterator>) -> ! { + let filename = LATEST_FILENAME.lock().unwrap().to_owned(); for e in errs { - Report::build(ReportKind::Error, filename, e.span().start) - .with_message(e.to_string()) - .with_label( - Label::new((filename.to_string(), e.span().into_range())) - .with_message(e.reason().to_string()) - .with_color(Color::Red), - ) - .with_labels(e.contexts().map(|(label, span)| { - Label::new((filename.to_string(), span.into_range())) - .with_message(format!("while parsing this {}", label)) - .with_color(Color::Yellow) - })) - .finish() - .print(sources([(filename.to_string(), text)])) - .unwrap(); + emit_ariadne_error( + Report::build(ReportKind::Error, &filename, e.span().start) + .with_message(e.to_string()) + .with_label( + Label::new((filename.to_string(), e.span().into_range())) + .with_message(e.reason().to_string()) + .with_color(Color::Red), + ) + .with_labels(e.contexts().map(|(label, span)| { + Label::new((filename.to_string(), span.into_range())) + .with_message(format!("while parsing this {}", label)) + .with_color(Color::Yellow) + })) + .finish(), + ) } exit(101); } @@ -964,7 +1002,7 @@ fn type_item<'a>( just(Token::KwType) .ignore_then(spanned(rust_type())) .then( - inner_item() + spanned(inner_item()) .repeated() .collect::>() .delimited_by(just(Token::BraceOpen), just(Token::BraceClose)), diff --git a/zngur-parser/src/tests.rs b/zngur-parser/src/tests.rs new file mode 100644 index 0000000..e363d8a --- /dev/null +++ b/zngur-parser/src/tests.rs @@ -0,0 +1,107 @@ +use std::panic::catch_unwind; + +use expect_test::{expect, Expect}; + +use crate::ParsedZngFile; + +fn check_success(zng: &str) { + let _ = ParsedZngFile::parse("main.zng", zng); +} + +pub struct ErrorText(pub String); + +fn check_fail(zng: &str, error: Expect) { + let r = catch_unwind(|| { + let _ = ParsedZngFile::parse("main.zng", zng); + }); + match r { + Ok(_) => panic!("Parsing succeeded but we expected fail"), + Err(e) => match e.downcast::() { + Ok(t) => error.assert_eq(&t.0), + Err(e) => std::panic::resume_unwind(e), + }, + } +} + +#[test] +fn parse_unit() { + check_success( + r#" +type () { + layout(size = 0, align = 1); + wellknown_traits(Copy); +} + "#, + ); +} + +#[test] +fn typo_in_wellknown_trait() { + check_fail( + r#" +type () { + layout(size = 0, align = 1); + welcome_traits(Copy); +} + "#, + expect![[r#" + Error: found 'welcome_traits' expected 'layout', '#', 'wellknown_traits', 'constructor', 'fn', or '}' + ╭─[main.zng:4:5] + │ + 4 │ welcome_traits(Copy); + │ ───────┬────── + │ ╰──────── found 'welcome_traits' expected 'layout', '#', 'wellknown_traits', 'constructor', 'fn', or '}' + ───╯ + "#]], + ); +} + +#[test] +fn multiple_layout_policies() { + check_fail( + r#" +type ::std::string::String { + layout(size = 24, align = 8); + #heap_allocated; +} + "#, + expect![[r#" + Error: Duplicate layout policy found + ╭─[main.zng:4:5] + │ + 4 │ #heap_allocated; + │ ───────┬─────── + │ ╰───────── Duplicate layout policy found + ───╯ + "#]], + ); +} + +#[test] +fn cpp_ref_should_not_need_layout_info() { + check_fail( + r#" +type crate::Way { + layout(size = 1, align = 2); + + #cpp_ref "::osmium::Way"; +} + "#, + expect![[r#" + Error: Duplicate layout policy found + ╭─[main.zng:3:5] + │ + 3 │ layout(size = 1, align = 2); + │ ─────────────┬───────────── + │ ╰─────────────── Duplicate layout policy found + ───╯ + "#]], + ); + check_success( + r#" +type crate::Way { + #cpp_ref "::osmium::Way"; +} + "#, + ); +} diff --git a/zngur/src/lib.rs b/zngur/src/lib.rs index 0d61bd5..3545520 100644 --- a/zngur/src/lib.rs +++ b/zngur/src/lib.rs @@ -42,7 +42,7 @@ impl Zngur { pub fn generate(self) { let path = self.zng_file; let file = std::fs::read_to_string(path).unwrap(); - let file = ParsedZngFile::parse("main.zng", &file, ZngurGenerator::build_from_zng); + let file = ZngurGenerator::build_from_zng(ParsedZngFile::parse("main.zng", &file)); let (rust, h, cpp) = file.render(); let rs_file_path = self.rs_file_path.expect("No rs file path provided");