Skip to content

Allow marking rust functions unsafe - #120

Open
adsnaider wants to merge 1 commit into
hkalbasi:mainfrom
adsnaider:push-rtntwzvrvzut
Open

Allow marking rust functions unsafe#120
adsnaider wants to merge 1 commit into
hkalbasi:mainfrom
adsnaider:push-rtntwzvrvzut

Conversation

@adsnaider

@adsnaider adsnaider commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Note: This change requires that functions specializations are explicit on the Rust side, whereas before you could leave the specialized type out of the .zng declaration since the compiler was smart enough to figure out those specializations.

Comment on lines +963 to +971
if is_safe {
let loops = (0..inputs.len()).map(|_| "loop {}").join(", ");
wln!(
self,
" const {{ let _verify_safety = || {{ #[allow(unreachable_code)] let _ = {}({}); }}; }};",
rust_name,
loops
);
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So no check for other direction?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I don't want to force that since I think there are reasons you might want to define a function as unsafe for the bridge since C++ doesn't have a borrow checker. I understand it doesn't do anything technically, but it's still good documentation

fn new() -> Vec<crate::KeyValuePair>;
fn push(&mut self, crate::KeyValuePair);
fn get(&self, usize) -> ::std::option::Option<&crate::KeyValuePair> deref [crate::KeyValuePair];
fn get<usize>(&self, usize) -> ::std::option::Option<&crate::KeyValuePair> deref [crate::KeyValuePair];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now these are going to be template specialization in the C++ side instead of overloaded function? Can we still have multiple instantiation? IIRC there were some difficulties on template specialization of the methods. An example which uses multiple instantiations of a single function may help.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. It works fine. Also, @hkalbasi, here's another situation you need to use __zngur_internal_assume_init since as far as I can tell there's no way to construct a RangeFull in C++ (the only constructor is RangeFull in rust which can't be mapped to C++'s)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants