From 146f7ad73f21816fc40e603669967259feb34b12 Mon Sep 17 00:00:00 2001 From: David Sankel Date: Mon, 31 Aug 2026 18:15:30 -0400 Subject: [PATCH 1/2] Add #cpp_heap_allocated directive, deprecate #cpp_value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #cpp_value's first argument is parsed but never used by codegen (it's overwritten before reaching generated code) — see #143. This adds #cpp_heap_allocated as a replacement without that dead parameter, deprecates #cpp_value (still functional, forwards to the same logic, emits an ariadne-rendered deprecation warning pointing at the directive), and updates the internal representation, docs, and examples to match. Co-Authored-By: Claude Sonnet 5 --- book/src/call_cpp_from_rust/opaque.md | 7 +- book/src/tutorial.md | 4 +- .../aggregation/aggregation.zng | 2 +- examples/tutorial_cpp/main.zng | 4 +- zngur-cli/src/main.rs | 3 +- zngur-def/src/lib.rs | 10 +- zngur-def/src/merge.rs | 17 +- zngur-generator/src/cpp.rs | 8 +- zngur-generator/src/lib.rs | 10 +- zngur-generator/src/rust.rs | 4 +- zngur-generator/templates/cpp_header.sptl | 24 +-- zngur-parser/Cargo.toml | 2 +- zngur-parser/src/lib.rs | 172 ++++++++++++------ zngur-parser/src/template_types.rs | 4 +- zngur-parser/src/tests.rs | 109 ++++++++++- zngur/src/lib.rs | 28 ++- 16 files changed, 302 insertions(+), 106 deletions(-) diff --git a/book/src/call_cpp_from_rust/opaque.md b/book/src/call_cpp_from_rust/opaque.md index 82bf3106..103d02f2 100644 --- a/book/src/call_cpp_from_rust/opaque.md +++ b/book/src/call_cpp_from_rust/opaque.md @@ -9,7 +9,7 @@ There are currently 3 kinds of opaque types that `zngur` is able to represent These types are made available to Rust with varying sets of restrictions and tradeoffs imposed based on your choice -For each of these types (aka. marked `#cpp_ref`, `#cpp_value`, or +For each of these types (aka. marked `#cpp_ref`, `#cpp_heap_allocated`, or `#cpp_stack_owned`), `zngur` will generate a new type within `pub mod cpp {}`. This module is where all generated opaque types live @@ -61,9 +61,12 @@ Those problem might be solved by the `extern type` language feature. ## Opaque Heap Allocated C++ Type -Keeping C++ objects in Rust using heap allocation is supported with `#cpp_value` types. +Keeping C++ objects in Rust using heap allocation is supported with `#cpp_heap_allocated` types. This is similar to `#cpp_ref` types. Look at the example `tutorial_cpp` for a usage example +> **NOTE**: `#cpp_value` is a deprecated alias for `#cpp_heap_allocated` and will +> emit a warning when used; new code should use `#cpp_heap_allocated`. + ### Semantics of Opaque Heap Allocated Types Heap allocated types are generated as a #[repr(C)] struct to a heap allocated pointer and destructor. diff --git a/book/src/tutorial.md b/book/src/tutorial.md index 8e973a10..019cb879 100644 --- a/book/src/tutorial.md +++ b/book/src/tutorial.md @@ -396,13 +396,13 @@ In the same directory, create a `main.zng` file with the following content: type crate::Inventory { #layout(size = 16, align = 8); - #cpp_value "0" "::cpp_inventory::Inventory"; + #cpp_heap_allocated "::cpp_inventory::Inventory"; } type crate::Item { #layout(size = 16, align = 8); - #cpp_value "0" "::cpp_inventory::Item"; + #cpp_heap_allocated "::cpp_inventory::Item"; } ``` diff --git a/examples/multiple_modules/aggregation/aggregation.zng b/examples/multiple_modules/aggregation/aggregation.zng index 7b4f8cd5..151e38d1 100644 --- a/examples/multiple_modules/aggregation/aggregation.zng +++ b/examples/multiple_modules/aggregation/aggregation.zng @@ -6,7 +6,7 @@ import "packet.zng"; type crate::StatsAccumulator { #layout(size = 16, align = 8); - #cpp_value "0" "::cpp_stats::StatsAccumulator"; + #cpp_heap_allocated "::cpp_stats::StatsAccumulator"; } extern "C++" { diff --git a/examples/tutorial_cpp/main.zng b/examples/tutorial_cpp/main.zng index db111528..7d6085e9 100644 --- a/examples/tutorial_cpp/main.zng +++ b/examples/tutorial_cpp/main.zng @@ -25,13 +25,13 @@ type ::std::result::Result<&str, ::std::str::Utf8Error> { type crate::Inventory { #layout(size = 16, align = 8); - #cpp_value "0" "::cpp_inventory::Inventory"; + #cpp_heap_allocated "::cpp_inventory::Inventory"; } type crate::Item { #layout(size = 16, align = 8); - #cpp_value "0" "::cpp_inventory::Item"; + #cpp_heap_allocated "::cpp_inventory::Item"; } type ::std::fmt::Result { diff --git a/zngur-cli/src/main.rs b/zngur-cli/src/main.rs index 900a2442..a7a81abf 100644 --- a/zngur-cli/src/main.rs +++ b/zngur-cli/src/main.rs @@ -172,7 +172,8 @@ fn main() { .with_cpp_file(cpp_file) .with_h_file(h_file) .with_rs_file(rs_file) - .with_zng_header_in_place_as(zng_header_in_place); + .with_zng_header_in_place_as(zng_header_in_place) + .with_warning_sink(|w| eprintln!("{w}")); if let Some(cpp_namespace) = cpp_namespace { zng = zng.with_cpp_namespace(&cpp_namespace); } diff --git a/zngur-def/src/lib.rs b/zngur-def/src/lib.rs index 8406040d..262778e8 100644 --- a/zngur-def/src/lib.rs +++ b/zngur-def/src/lib.rs @@ -146,7 +146,13 @@ pub struct ZngurMethodDetails { } #[derive(Debug, PartialEq, Eq, Clone)] -pub struct CppValue(pub String, pub String); +pub struct CppHeapAllocated(pub String); + +#[derive(Debug)] +pub struct CppHeapAllocatedData { + pub bridge_fn: String, + pub cpp_type: String, +} #[derive(Debug, PartialEq, Eq, Clone)] pub struct CppRef(pub String); @@ -174,7 +180,7 @@ pub struct ZngurType { pub constructor: Option, pub variants: Vec, pub fields: Vec, - pub cpp_value: Option, + pub cpp_heap_allocated: Option, pub cpp_ref: Option, pub cpp_stack_owned: Option, } diff --git a/zngur-def/src/merge.rs b/zngur-def/src/merge.rs index cb302347..1ec61bab 100644 --- a/zngur-def/src/merge.rs +++ b/zngur-def/src/merge.rs @@ -1,6 +1,6 @@ use crate::{ - AdditionalIncludes, ConvertPanicToException, CppRef, CppStackOwned, CppValue, LayoutPolicy, - ZngurConstructor, ZngurExternCppFn, ZngurExternCppImpl, ZngurField, ZngurFn, + AdditionalIncludes, ConvertPanicToException, CppHeapAllocated, CppRef, CppStackOwned, + LayoutPolicy, ZngurConstructor, ZngurExternCppFn, ZngurExternCppImpl, ZngurField, ZngurFn, ZngurMethodDetails, ZngurSpec, ZngurTrait, ZngurType, ZngurVariant, }; @@ -148,7 +148,8 @@ impl Merge for ZngurType { )); } - self.cpp_value.merge(&mut into.cpp_value)?; + self.cpp_heap_allocated + .merge(&mut into.cpp_heap_allocated)?; self.cpp_ref.merge(&mut into.cpp_ref)?; self.cpp_stack_owned.merge(&mut into.cpp_stack_owned)?; @@ -189,14 +190,16 @@ impl Merge for ZngurTrait { } } -impl Merge for CppValue { +impl Merge for CppHeapAllocated { /// Writes the partial union of `self` and `into` to the latter. /// - /// There is no meaningful way to merge different CppValues, but we allow - /// merging the same CppValue from different sources. + /// There is no meaningful way to merge different CppHeapAllocateds, but we allow + /// merging the same CppHeapAllocated from different sources. fn merge(self, into: &mut Self) -> MergeResult { if self != *into { - return Err(MergeFailure::Conflict("Cpp value mismatch".to_string())); + return Err(MergeFailure::Conflict( + "Cpp heap allocated mismatch".to_string(), + )); } Ok(()) } diff --git a/zngur-generator/src/cpp.rs b/zngur-generator/src/cpp.rs index 83e19033..4d24ac32 100644 --- a/zngur-generator/src/cpp.rs +++ b/zngur-generator/src/cpp.rs @@ -5,7 +5,9 @@ use std::{ use indexmap::IndexMap; use itertools::Itertools; -use zngur_def::{CppRef, CppStackOwned, CppValue, RustTrait, ZngurFieldData, ZngurMethodReceiver}; +use zngur_def::{ + CppHeapAllocatedData, CppRef, CppStackOwned, RustTrait, ZngurFieldData, ZngurMethodReceiver, +}; use crate::{ ZngurWellknownTraitData, @@ -525,7 +527,7 @@ pub struct CppTypeDefinition { pub from_trait: Option, pub from_trait_ref: Option, pub wellknown_traits: Vec, - pub cpp_value: Option, + pub cpp_heap_allocated: Option, pub cpp_ref: Option, pub cpp_stack_owned: Option, } @@ -659,7 +661,7 @@ impl Default for CppTypeDefinition { wellknown_traits: vec![], from_trait: None, from_trait_ref: None, - cpp_value: None, + cpp_heap_allocated: None, cpp_ref: None, cpp_stack_owned: None, } diff --git a/zngur-generator/src/lib.rs b/zngur-generator/src/lib.rs index ef97bbed..2a261a15 100644 --- a/zngur-generator/src/lib.rs +++ b/zngur-generator/src/lib.rs @@ -161,7 +161,7 @@ impl ZngurGenerator { "# )); } - if ty_def.cpp_value.is_some() { + if ty_def.cpp_heap_allocated.is_some() { let type_name = ty.to_string().split("::").last().unwrap().to_string(); cpp_mod_content.push_str(&format!( r#" @@ -325,9 +325,11 @@ impl ZngurGenerator { fields, methods: cpp_methods, wellknown_traits, - cpp_value: ty_def.cpp_value.map(|mut cpp_value| { - cpp_value.0 = rust_file.add_cpp_value_bridge(&ty); - cpp_value + cpp_heap_allocated: ty_def.cpp_heap_allocated.map(|cpp_heap_allocated| { + CppHeapAllocatedData { + bridge_fn: rust_file.add_cpp_heap_allocated_bridge(&ty), + cpp_type: cpp_heap_allocated.0, + } }), cpp_ref: ty_def.cpp_ref, cpp_stack_owned: ty_def.cpp_stack_owned, diff --git a/zngur-generator/src/rust.rs b/zngur-generator/src/rust.rs index a4fd1cd5..e700a33b 100644 --- a/zngur-generator/src/rust.rs +++ b/zngur-generator/src/rust.rs @@ -1099,9 +1099,9 @@ pub {}fn {rust_name}("#, mangled_name } - pub fn add_cpp_value_bridge(&mut self, ty: &RustType) -> String { + pub fn add_cpp_heap_allocated_bridge(&mut self, ty: &RustType) -> String { let type_name = ty.to_string().split("::").last().unwrap().to_string(); - let mangled_name = self.mangle_name(&format!("{ty}_cpp_value")); + let mangled_name = self.mangle_name(&format!("{ty}_cpp_heap_allocated")); w!( self, r#" diff --git a/zngur-generator/templates/cpp_header.sptl b/zngur-generator/templates/cpp_header.sptl index 4d370396..1d42df40 100644 --- a/zngur-generator/templates/cpp_header.sptl +++ b/zngur-generator/templates/cpp_header.sptl @@ -78,8 +78,8 @@ extern "C" { uint32_t {{ discriminant }} (uint8_t* i) noexcept; {% endif %} - {% if let Some(cpp_value) = td.cpp_value %} - ::{{ self.namespace }}::ZngurCppOpaqueOwnedObject* {{ cpp_value.0 }}(uint8_t*); + {% if let Some(cpp_heap_allocated) = td.cpp_heap_allocated %} + ::{{ self.namespace }}::ZngurCppOpaqueOwnedObject* {{ cpp_heap_allocated.bridge_fn }}(uint8_t*); {% endif %} {% if !td.layout.is_stack_allocated() && !td.layout.is_only_by_ref() %} @@ -368,9 +368,9 @@ namespace {{ self.namespace }} { {{ td.render_make_box(name, self.namespace, self.crate_name) }} - {% if let Some(cpp_value) = td.cpp_value %} - inline {{ cpp_value.1 }}& cpp() { - return (*{{ cpp_value.0 }}(::{{ self.namespace }}::__zngur_internal_data_ptr(*this))).as_cpp< {{ cpp_value.1 }} >(); + {% if let Some(cpp_heap_allocated) = td.cpp_heap_allocated %} + inline {{ cpp_heap_allocated.cpp_type }}& cpp() { + return (*{{ cpp_heap_allocated.bridge_fn }}(::{{ self.namespace }}::__zngur_internal_data_ptr(*this))).as_cpp< {{ cpp_heap_allocated.cpp_type }} >(); } inline {{ name }}(::{{ self.namespace }}::ZngurCppOpaqueOwnedObject&& o) noexcept { @@ -382,7 +382,7 @@ namespace {{ self.namespace }} { template static inline {{ name }} build(Args&&... args) { - return {{ name }}(::{{ self.namespace }}::ZngurCppOpaqueOwnedObject::build< {{ cpp_value.1 }} >(::std::forward(args)...)); + return {{ name }}(::{{ self.namespace }}::ZngurCppOpaqueOwnedObject::build< {{ cpp_heap_allocated.cpp_type }} >(::std::forward(args)...)); } {% endif %} @@ -704,9 +704,9 @@ namespace {{ self.namespace }} { {{ td.render_make_box_ref(td.ty.path.name(), self.namespace, self.crate_name) }} - {% if let Some(cpp_value) = td.cpp_value %} - inline {{ cpp_value.1 }}& cpp() { - return (*{{ cpp_value.0 }}(reinterpret_cast(__zngur_data))).as_cpp< {{ cpp_value.1 }} >(); + {% if let Some(cpp_heap_allocated) = td.cpp_heap_allocated %} + inline {{ cpp_heap_allocated.cpp_type }}& cpp() { + return (*{{ cpp_heap_allocated.bridge_fn }}(reinterpret_cast(__zngur_data))).as_cpp< {{ cpp_heap_allocated.cpp_type }} >(); } {% endif %} @@ -928,9 +928,9 @@ namespace {{ self.namespace }} { {{ td.render_make_box_ref_only(td.ty.path.name(), self.namespace, self.crate_name) }} - {% if let Some(cpp_value) = td.cpp_value %} - inline {{ cpp_value.1 }}& cpp() { - return (*{{ cpp_value.0 }}(reinterpret_cast(__zngur_data))).as_cpp< {{ cpp_value.1 }} >(); + {% if let Some(cpp_heap_allocated) = td.cpp_heap_allocated %} + inline {{ cpp_heap_allocated.cpp_type }}& cpp() { + return (*{{ cpp_heap_allocated.bridge_fn }}(reinterpret_cast(__zngur_data))).as_cpp< {{ cpp_heap_allocated.cpp_type }} >(); } {% endif %} diff --git a/zngur-parser/Cargo.toml b/zngur-parser/Cargo.toml index 524b58ce..5aa3e6cc 100644 --- a/zngur-parser/Cargo.toml +++ b/zngur-parser/Cargo.toml @@ -13,8 +13,8 @@ license.workspace = true ariadne = "0.3.0" chumsky = { version = "=1.0.0-alpha.8", features = [] } itertools = "0.11" +strip-ansi-escapes = "0.2.0" zngur-def = { version = "=0.11.0", 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 b84e75dc..a64d4d8f 100644 --- a/zngur-parser/src/lib.rs +++ b/zngur-parser/src/lib.rs @@ -12,7 +12,7 @@ use chumsky::{input::MapExtra, prelude::*}; use itertools::{Either, Itertools}; use zngur_def::{ - AdditionalIncludes, ConvertPanicToException, CppRef, CppStackOwned, CppValue, Import, + AdditionalIncludes, ConvertPanicToException, CppHeapAllocated, CppRef, CppStackOwned, Import, LayoutPolicy, Merge, MergeFailure, ModuleImport, Mutability, PrimitiveRustType, RustPathAndGenerics, RustTrait, RustType, TypeVar, ZngurConstructor, ZngurExternCppFn, ZngurExternCppImpl, ZngurField, ZngurFn, ZngurMethod, ZngurMethodDetails, ZngurMethodReceiver, @@ -381,6 +381,9 @@ enum ParsedTypeItem<'a> { field: &'a str, cpp_type: &'a str, }, + CppHeapAllocated { + cpp_type: &'a str, + }, CppRef { cpp_type: &'a str, }, @@ -497,7 +500,7 @@ impl ProcessedItem<'_> { let mut layout = None; let mut layout_span = None; let mut exhaustive = true; - let mut cpp_value = None; + let mut cpp_heap_allocated = None; let mut cpp_ref = None; let mut cpp_stack_owned = None; let mut to_process = items; @@ -664,8 +667,15 @@ impl ProcessedItem<'_> { cpp_name: cpp_name.map(|s| s.to_owned()), }); } - ParsedTypeItem::CppValue { field, cpp_type } => { - cpp_value = Some(CppValue(field.to_owned(), cpp_type.to_owned())); + ParsedTypeItem::CppValue { field: _, cpp_type } => { + ctx.add_warning_str( + "#cpp_value is deprecated; use #cpp_heap_allocated instead", + item_span, + ); + cpp_heap_allocated = Some(CppHeapAllocated(cpp_type.to_owned())); + } + ParsedTypeItem::CppHeapAllocated { cpp_type } => { + cpp_heap_allocated = Some(CppHeapAllocated(cpp_type.to_owned())); } ParsedTypeItem::CppRef { cpp_type } => { match layout_span { @@ -715,7 +725,7 @@ impl ProcessedItem<'_> { .collect::>(); if let Some(is_unsized) = is_unsized { if let Some(span) = layout_span { - ctx.add_report( + ctx.add_fatal_report( Report::build( ReportKind::Error, ctx.filename().to_string(), @@ -751,7 +761,7 @@ impl ProcessedItem<'_> { constructor, variants, fields, - cpp_value, + cpp_heap_allocated, cpp_ref, cpp_stack_owned, }; @@ -956,11 +966,17 @@ impl ParsedRustPathAndGenerics<'_> { } } +/// A diagnostic report, tagged with whether it should abort the parse. +struct ReportEntry<'b> { + fatal: bool, + report: Report<'b, (String, std::ops::Range)>, +} + struct ParseContext<'a, 'b> { path: std::path::PathBuf, text: &'a str, depth: usize, - reports: Vec)>>, + reports: Vec>, source_cache: std::collections::HashMap, /// All .zng files processed during parsing (main file + imports) processed_files: Vec, @@ -1003,13 +1019,38 @@ impl<'a, 'b> ParseContext<'a, 'b> { self.path.file_name().unwrap().to_str().unwrap() } - fn add_report(&mut self, report: Report<'b, (String, std::ops::Range)>) { - self.reports.push(report); + /// Every known source file (this file plus every imported file already + /// processed), paired with its text, for ariadne's `sources()` cache. Needed so + /// that a report whose span points into an imported file can still be rendered + /// correctly after that file's `ParseContext` has been merged into this one. + fn all_sources(&self) -> impl Iterator { + [(self.filename().to_string(), self.text)] + .into_iter() + .chain(self.source_cache.iter().map(|(path, text)| { + ( + path.file_name().unwrap().to_str().unwrap().to_string(), + text.as_str(), + ) + })) + } + + /// Renders a single report to text (stripped of ANSI color codes). + fn render_report(&self, report: &Report<'b, (String, std::ops::Range)>) -> String { + let mut buf = Vec::new(); + report.write(sources(self.all_sources()), &mut buf).unwrap(); + String::from_utf8(strip_ansi_escapes::strip(buf)).unwrap() + } + + fn add_fatal_report(&mut self, report: Report<'b, (String, std::ops::Range)>) { + self.reports.push(ReportEntry { + fatal: true, + report, + }); } fn add_errors<'err_src>(&mut self, errs: impl Iterator>) { let filename = self.filename().to_string(); self.reports.extend(errs.map(|e| { - Report::build(ReportKind::Error, &filename, e.span().start) + let report = Report::build(ReportKind::Error, &filename, e.span().start) .with_message(e.to_string()) .with_label( Label::new((filename.clone(), e.span().into_range())) @@ -1021,7 +1062,11 @@ impl<'a, 'b> ParseContext<'a, 'b> { .with_message(format!("while parsing this {}", label)) .with_color(Color::Yellow) })) - .finish() + .finish(); + ReportEntry { + fatal: true, + report, + } })); } @@ -1029,6 +1074,24 @@ impl<'a, 'b> ParseContext<'a, 'b> { self.add_errors([Rich::custom(span, error)].into_iter()); } + /// Records a non-fatal warning as an ariadne diagnostic pointing at `span`. + /// Rendering is deferred until dispatch time (see `render_report`). + fn add_warning_str(&mut self, warning: &str, span: Span) { + let filename = self.filename().to_string(); + let report = Report::build(ReportKind::Warning, &filename, span.start) + .with_message(warning) + .with_label( + Label::new((filename.clone(), span.into_range())) + .with_message(warning) + .with_color(Color::Yellow), + ) + .finish(); + self.reports.push(ReportEntry { + fatal: false, + report, + }); + } + fn consume_from(&mut self, mut other: ParseContext<'_, 'b>) { self.processed_files.append(&mut other.processed_files); self.reports.extend(other.reports); @@ -1038,33 +1101,17 @@ impl<'a, 'b> ParseContext<'a, 'b> { } fn has_errors(&self) -> bool { - !self.reports.is_empty() + self.reports.iter().any(|entry| entry.fatal) } #[cfg(test)] fn emit_ariadne_errors(&self) -> ! { let mut r = Vec::::new(); - for err in &self.reports { - err.write( - sources( - [(self.filename().to_string(), self.text)] - .into_iter() - .chain( - self.source_cache - .iter() - .map(|(path, text)| { - ( - path.file_name().unwrap().to_str().unwrap().to_string(), - text.as_str(), - ) - }) - .collect::>() - .into_iter(), - ), - ), - &mut r, - ) - .unwrap(); + for entry in self.reports.iter().filter(|entry| entry.fatal) { + entry + .report + .write(sources(self.all_sources()), &mut r) + .unwrap(); } std::panic::resume_unwind(Box::new(tests::ErrorText({ let s = String::from_utf8(strip_ansi_escapes::strip(r)).unwrap(); @@ -1075,24 +1122,8 @@ impl<'a, 'b> ParseContext<'a, 'b> { #[cfg(not(test))] fn emit_ariadne_errors(&self) -> ! { - for err in &self.reports { - err.eprint(sources( - [(self.filename().to_string(), self.text)] - .into_iter() - .chain( - self.source_cache - .iter() - .map(|(path, text)| { - ( - path.file_name().unwrap().to_str().unwrap().to_string(), - text.as_str(), - ) - }) - .collect::>() - .into_iter(), - ), - )) - .unwrap(); + for entry in self.reports.iter().filter(|entry| entry.fatal) { + entry.report.eprint(sources(self.all_sources())).unwrap(); } exit(101); } @@ -1177,7 +1208,7 @@ impl<'a> ParsedZngFile<'a> { // TODO: emit a better error. How should we get a span here? // I'd like to avoid putting a ParsedImportPath in ZngurSpec, and // also not have to pass a filename to add_to_zngur_spec. - ctx.add_report( + ctx.add_fatal_report( Report::build(ReportKind::Error, ctx.filename(), 0) .with_message(format!( "Import path not found: {}", @@ -1192,7 +1223,14 @@ impl<'a> ParsedZngFile<'a> { } /// Parse a .zng file and return both the spec and list of all processed files. - pub fn parse(path: std::path::PathBuf, cfg: Box) -> ParseResult { + /// + /// `warning_sink` is called once per deprecation or other non-fatal warning + /// produced while parsing. + pub fn parse( + path: std::path::PathBuf, + cfg: Box, + warning_sink: &dyn Fn(&str), + ) -> ParseResult { let mut zngur = ZngurSpecBuilder::default(); zngur.spec.rust_cfg.extend(cfg.get_cfg_pairs()); zngur.spec.rust_cfg.sort(); @@ -1200,9 +1238,12 @@ impl<'a> ParsedZngFile<'a> { let mut ctx = ParseContext::new(path.clone(), &text, cfg.clone_box()); Self::parse_into(&mut zngur, &mut ctx, &DefaultImportResolver); let spec = zngur.to_zngur(&mut ctx); + for entry in ctx.reports.iter().filter(|entry| !entry.fatal) { + warning_sink(&ctx.render_report(&entry.report)); + } if ctx.has_errors() { // add report of cfg values used - ctx.add_report( + ctx.add_fatal_report( Report::build( ReportKind::Custom("cfg values", ariadne::Color::Green), path.file_name().unwrap_or_default().to_string_lossy(), @@ -1230,8 +1271,12 @@ impl<'a> ParsedZngFile<'a> { /// Parse a .zng file from a string. Mainly useful for testing. #[cfg(test)] - pub fn parse_str(text: &str, cfg: impl RustCfgProvider + 'static) -> ParseResult { - Self::parse_str_with_resolver(text, cfg, &DefaultImportResolver) + pub fn parse_str( + text: &str, + cfg: impl RustCfgProvider + 'static, + warning_sink: impl FnMut(&str), + ) -> ParseResult { + Self::parse_str_with_resolver(text, cfg, &DefaultImportResolver, warning_sink) } #[cfg(test)] @@ -1239,11 +1284,15 @@ impl<'a> ParsedZngFile<'a> { text: &str, cfg: impl RustCfgProvider + 'static, resolver: &impl ImportResolver, + mut warning_sink: impl FnMut(&str), ) -> ParseResult { let mut zngur = ZngurSpecBuilder::default(); let mut ctx = ParseContext::new(std::path::PathBuf::from("test.zng"), text, Box::new(cfg)); Self::parse_into(&mut zngur, &mut ctx, resolver); let spec = zngur.to_zngur(&mut ctx); + for entry in ctx.reports.iter().filter(|entry| !entry.fatal) { + warning_sink(&ctx.render_report(&entry.report)); + } if ctx.has_errors() { ctx.emit_ariadne_errors(); } @@ -1381,7 +1430,7 @@ impl ZngurSpecBuilder { ); if let Err(e) = template_match.merge(ty) { let MergeFailure::Conflict(e) = e; - ctx.add_report( + ctx.add_fatal_report( Report::build(ReportKind::Error, &template.filename, 0) .with_message(format!( "Failed to apply template {} to type {}: {}", @@ -1427,7 +1476,7 @@ impl ZngurSpecBuilder { .with_color(Color::Blue), ); } - ctx.add_report(report.finish()); + ctx.add_fatal_report(report.finish()); } } spec @@ -2016,6 +2065,12 @@ fn inner_type_item<'a>() field: x.0, cpp_type: x.1, }); + let cpp_heap_allocated = just(Token::Sharp) + .then(just(Token::Ident("cpp_heap_allocated"))) + .ignore_then(select! { + Token::Str(c) => c, + }) + .map(|cpp_type| ParsedTypeItem::CppHeapAllocated { cpp_type }); let cpp_ref = just(Token::Sharp) .then(just(Token::Ident("cpp_ref"))) .ignore_then(select! { @@ -2057,6 +2112,7 @@ fn inner_type_item<'a>() constructor.boxed(), field.boxed(), cpp_value.boxed(), + cpp_heap_allocated.boxed(), cpp_ref.boxed(), cpp_stack_owned.boxed(), method() diff --git a/zngur-parser/src/template_types.rs b/zngur-parser/src/template_types.rs index 8ff368a1..0d2dcf48 100644 --- a/zngur-parser/src/template_types.rs +++ b/zngur-parser/src/template_types.rs @@ -236,7 +236,7 @@ pub fn try_match_template(ty: &RustType, template: &ZngurType) -> Option Option panic!("Parsing succeeded but we expected fail"), @@ -33,7 +33,7 @@ fn check_fail_with_cfg( error: Expect, ) { let r = catch_unwind(|| { - let _ = ParsedZngFile::parse_str(zng, cfg); + let _ = ParsedZngFile::parse_str(zng, cfg, |_| {}); }); match r { Ok(_) => panic!("Parsing succeeded but we expected fail"), @@ -46,7 +46,7 @@ fn check_fail_with_cfg( fn check_import_fail(zng: &str, error: Expect, resolver: &MockFilesystem) { let r = catch_unwind(|| { - let _ = ParsedZngFile::parse_str_with_resolver(zng, NullCfg, resolver); + let _ = ParsedZngFile::parse_str_with_resolver(zng, NullCfg, resolver, |_| {}); }); match r { @@ -63,7 +63,7 @@ fn catch_parse_fail( zng: &str, cfg: impl RustCfgProvider + std::panic::UnwindSafe + 'static, ) -> crate::ParseResult { - let r = catch_unwind(move || ParsedZngFile::parse_str(zng, cfg)); + let r = catch_unwind(move || ParsedZngFile::parse_str(zng, cfg, |_| {})); match r { Ok(r) => r, @@ -183,6 +183,93 @@ type crate::Way { ); } +#[test] +fn cpp_heap_allocated_directive_parses() { + let result = ParsedZngFile::parse_str( + r#" +type crate::Way { + #layout(size = 16, align = 8); + #cpp_heap_allocated "::osmium::Way"; +} + "#, + NullCfg, + |_| {}, + ); + let ty = result.spec.types.first().expect("no type parsed"); + assert_eq!( + ty.cpp_heap_allocated, + Some(CppHeapAllocated("::osmium::Way".to_owned())), + ); +} + +#[test] +fn cpp_value_emits_deprecation_warning_and_forwards_to_cpp_heap_allocated() { + let mut warnings = Vec::new(); + let result = ParsedZngFile::parse_str( + r#" +type crate::Way { + #layout(size = 16, align = 8); + #cpp_value "0" "::osmium::Way"; +} + "#, + NullCfg, + |w| warnings.push(w.to_owned()), + ); + assert_eq!(warnings.len(), 1); + expect![[r#" + Warning: #cpp_value is deprecated; use #cpp_heap_allocated instead + ╭─[test.zng:4:5] + │ + 4 │ #cpp_value "0" "::osmium::Way"; + │ ───────────────┬─────────────── + │ ╰───────────────── #cpp_value is deprecated; use #cpp_heap_allocated instead + ───╯ + "#]].assert_eq(&warnings[0]); + let ty = result.spec.types.first().expect("no type parsed"); + assert_eq!( + ty.cpp_heap_allocated, + Some(CppHeapAllocated("::osmium::Way".to_owned())), + ); +} + +#[test] +fn cpp_value_and_cpp_heap_allocated_merge_as_equivalent_across_files() { + let resolver = MockFilesystem::new(vec![( + "./a.zng", + r#" +type crate::Way { + #layout(size = 16, align = 8); + #cpp_value "0" "::osmium::Way"; +} + "#, + )]); + + let mut warnings = Vec::new(); + let parsed = ParsedZngFile::parse_str_with_resolver( + r#" +merge "./a.zng"; +type crate::Way { + #cpp_heap_allocated "::osmium::Way"; +} + "#, + NullCfg, + &resolver, + |w| warnings.push(w.to_owned()), + ); + // The #cpp_value directive that triggered this warning lives in the imported + // a.zng, not the root file, so the rendered warning must show a.zng's own + // source snippet -- this exercises the cross-file source lookup used when + // rendering a warning that was merged in from an imported file's context. + assert_eq!(warnings.len(), 1); + assert!(warnings[0].contains("a.zng")); + assert!(warnings[0].contains(r#"#cpp_value "0" "::osmium::Way";"#)); + let ty = parsed.spec.types.first().expect("no type parsed"); + assert_eq!( + ty.cpp_heap_allocated, + Some(CppHeapAllocated("::osmium::Way".to_owned())), + ); +} + macro_rules! assert_ty_path { ($path_expected:expr, $ty:expr) => {{ let RustType::Adt(RustPathAndGenerics { path: p, .. }) = $ty else { @@ -202,6 +289,7 @@ type MyString { } "#, NullCfg, + |_| {}, ); let ty = parsed.spec.types.first().expect("no type parsed"); let RustType::Adt(RustPathAndGenerics { path: p, .. }) = &ty.ty else { @@ -223,6 +311,7 @@ mod crate { } "#, NullCfg, + |_| {}, ); let ty = parsed.spec.types.first().expect("no type parsed"); let RustType::Adt(RustPathAndGenerics { path: p, .. }) = &ty.ty else { @@ -246,6 +335,7 @@ type Option { } "#, NullCfg, + |_| {}, ); let ty = parsed.spec.types.first().expect("no type parsed"); @@ -311,6 +401,7 @@ type Example { "#, NullCfg, &resolver, + |_| {}, ); assert_eq!(parsed.spec.types.len(), 2); } @@ -571,6 +662,7 @@ type A { } "#, NullCfg, + |_| {}, ); // Should have exactly one file (test.zng) assert_eq!(parsed.processed_files.len(), 1); @@ -600,6 +692,7 @@ type Main { "#, NullCfg, &resolver, + |_| {}, ); // Should have two files: main (test.zng) + imported assert_eq!(parsed.processed_files.len(), 2); @@ -635,6 +728,7 @@ type Main { "#, NullCfg, &resolver, + |_| {}, ); // Should have four files: main + a + b + c assert_eq!(parsed.processed_files.len(), 4); @@ -1027,6 +1121,7 @@ fn module_import_parser_test() { import "module.zng"; "#, crate::cfg::NullCfg, + |_| {}, ); assert_eq!(parsed.spec.imported_modules.len(), 1); assert_eq!( @@ -1085,6 +1180,7 @@ fn cpp_additional_includes() { " "#, crate::cfg::NullCfg, + |_| {}, ); assert_eq!( parsed.spec.additional_includes.0, @@ -1099,6 +1195,7 @@ fn cpp_additional_includes() { "# "##, crate::cfg::NullCfg, + |_| {}, ); assert_eq!( parsed.spec.additional_includes.0, diff --git a/zngur/src/lib.rs b/zngur/src/lib.rs index 3da9ea5f..bc0543e6 100644 --- a/zngur/src/lib.rs +++ b/zngur/src/lib.rs @@ -38,6 +38,7 @@ pub struct Zngur { zng_header_in_place: bool, zng_h_file_path: Option, crate_name: Option, + warning_sink: Option>, } impl Zngur { @@ -54,6 +55,7 @@ impl Zngur { zng_header_in_place: false, zng_h_file_path: None, crate_name: None, + warning_sink: None, } } @@ -133,9 +135,33 @@ impl Zngur { self } + /// Set how deprecation and other non-fatal warnings produced while parsing the + /// `.zng` file are reported. Each warning is a rendered (possibly multi-line) + /// diagnostic, already labeled (e.g. `Warning: ...`), pointing at the relevant + /// source location. + /// + /// By default (if this is never called), warnings are printed to stdout using + /// Cargo's `cargo:warning=` build-script convention, which is appropriate when + /// [`generate`](Self::generate) is called from a build script. Callers that are + /// not running inside a build script (e.g. a CLI) should call this to print + /// warnings some other way, e.g. `.with_warning_sink(|w| eprintln!("{w}"))`. + pub fn with_warning_sink(mut self, sink: impl Fn(&str) + 'static) -> Self { + self.warning_sink = Some(Box::new(sink)); + self + } + pub fn generate(self) { + let warning_sink = self.warning_sink.unwrap_or_else(|| { + Box::new(|w: &str| { + // `cargo:warning=` is a line-based directive, so a multi-line warning + // needs the prefix repeated on every line to stay visible to Cargo. + for line in w.lines() { + println!("cargo:warning={line}"); + } + }) + }); let rust_cfg = self.rust_cfg.unwrap_or_else(|| Box::new(NullCfg)); - let parse_result = ParsedZngFile::parse(self.zng_file, rust_cfg); + let parse_result = ParsedZngFile::parse(self.zng_file, rust_cfg, &*warning_sink); let crate_name = self .crate_name .or_else(|| std::env::var("CARGO_PKG_NAME").ok()) From fbf9f4caef12e773d2b804756fb91087a7fb76de Mon Sep 17 00:00:00 2001 From: David Sankel Date: Mon, 31 Aug 2026 18:26:53 -0400 Subject: [PATCH 2/2] Fix CI spell-check failure in merge.rs doc comment cspell flagged "Allocateds" (an invented plural of CppHeapAllocated) as an unknown word, unlike "CppRefs" nearby, whose "Refs" is a real word. Reworded to avoid pluralizing the type name. Co-Authored-By: Claude Sonnet 5 --- zngur-def/src/merge.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zngur-def/src/merge.rs b/zngur-def/src/merge.rs index 1ec61bab..b6afd2a7 100644 --- a/zngur-def/src/merge.rs +++ b/zngur-def/src/merge.rs @@ -193,8 +193,8 @@ impl Merge for ZngurTrait { impl Merge for CppHeapAllocated { /// Writes the partial union of `self` and `into` to the latter. /// - /// There is no meaningful way to merge different CppHeapAllocateds, but we allow - /// merging the same CppHeapAllocated from different sources. + /// There is no meaningful way to merge different CppHeapAllocated values, but we + /// allow merging the same CppHeapAllocated value from different sources. fn merge(self, into: &mut Self) -> MergeResult { if self != *into { return Err(MergeFailure::Conflict(