Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow function aliases #218

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions demo-cxx/demo.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,18 @@ ThingC::ThingC(std::string appname) : appname(std::move(appname)) {}

ThingC::~ThingC() { std::cout << "done with ThingC" << std::endl; }

void ThingC::camelCaseMethod() const { std::cout << "camelCaseMethod" << std::endl; }
void ThingC::overloadedMethod(int x) const { std::cout << "overloadedMethod: int x = " << x << std::endl; }
void ThingC::overloadedMethod(float x) const { std::cout << "overloadedMethod: float x = " << x << std::endl; }

std::unique_ptr<ThingC> make_demo(rust::Str appname) {
return std::make_unique<ThingC>(std::string(appname));
}

void camelCaseFunction() { std::cout << "camelCaseFunction" << std::endl; }
void overloadedFunction(int x) { std::cout << "overloadedFunction: int x = " << x << std::endl; }
void overloadedFunction(float x) { std::cout << "overloadedFunction: float x = " << x << std::endl; }

const std::string &get_name(const ThingC &thing) { return thing.appname; }

void do_thing(SharedThing state) { print_r(*state.y); }
Expand Down
7 changes: 7 additions & 0 deletions demo-cxx/demo.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,18 @@ class ThingC {
ThingC(std::string appname);
~ThingC();

void camelCaseMethod() const;
void overloadedMethod(int x) const;
void overloadedMethod(float x) const;

std::string appname;
};

struct SharedThing;

void camelCaseFunction();
void overloadedFunction(int x);
void overloadedFunction(float x);
std::unique_ptr<ThingC> make_demo(rust::Str appname);
const std::string &get_name(const ThingC &thing);
void do_thing(SharedThing state);
Expand Down
21 changes: 20 additions & 1 deletion demo-rs/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@ mod ffi {
include!("demo-cxx/demo.h");

type ThingC;
#[alias(snake_case_method)]
fn camelCaseMethod(&self);
#[alias(snake_case_function)]
fn camelCaseFunction();
#[alias(i32_method)]
fn overloadedMethod(&self, x: i32);
#[alias(f32_method)]
fn overloadedMethod(&self, x: f32);
#[alias(i32_function)]
fn overloadedFunction(x: i32);
#[alias(f32_function)]
fn overloadedFunction(x: f32);
fn make_demo(appname: &str) -> UniquePtr<ThingC>;
fn get_name(thing: &ThingC) -> &CxxString;
fn do_thing(state: SharedThing);
Expand All @@ -30,7 +42,14 @@ fn print_r(r: &ThingR) {
fn main() {
let x = ffi::make_demo("demo of cxx::bridge");
println!("this is a {}", ffi::get_name(x.as_ref().unwrap()));


x.snake_case_method();
ffi::snake_case_function();
x.i32_method(10);
x.f32_method(10.5);
ffi::i32_function(10);
ffi::f32_function(10.5);

ffi::do_thing(ffi::SharedThing {
z: 222,
y: Box::new(ThingR(333)),
Expand Down
14 changes: 8 additions & 6 deletions gen/src/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,9 +433,10 @@ fn write_cxx_function_shim(out: &mut OutFile, efn: &ExternFn, types: &Types) {
writeln!(out, ") noexcept {{");
write!(out, " ");
write_return_type(out, &efn.ret);
let ident = if let Some(alias) = &efn.alias { alias } else { &efn.ident };
Copy link
Owner

Choose a reason for hiding this comment

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

I don't like this implementation of functions having an "ident" and an optional "alias", and especially counting on each use site to remember whether they should be checking for presence of the alias.

In my mind every function should have a non-optional rust name and non-optional c++ name. That's how we should store them post-parsing. Every place which uses of a function name will pick which of the two they want.

match &efn.receiver {
None => write!(out, "(*{}$)(", efn.ident),
Some(receiver) => write!(out, "({}::*{}$)(", receiver.ty, efn.ident),
None => write!(out, "(*{}$)(", ident),
Some(receiver) => write!(out, "({}::*{}$)(", receiver.ty, ident),
}
for (i, arg) in efn.args.iter().enumerate() {
if i > 0 {
Expand Down Expand Up @@ -478,8 +479,8 @@ fn write_cxx_function_shim(out: &mut OutFile, efn: &ExternFn, types: &Types) {
_ => {}
}
match &efn.receiver {
None => write!(out, "{}$(", efn.ident),
Some(_) => write!(out, "(self.*{}$)(", efn.ident),
None => write!(out, "{}$(", ident),
Some(_) => write!(out, "(self.*{}$)(", ident),
}
for (i, arg) in efn.args.iter().enumerate() {
if i > 0 {
Expand Down Expand Up @@ -612,9 +613,10 @@ fn write_rust_function_shim(out: &mut OutFile, efn: &ExternFn, types: &Types) {
for line in efn.doc.to_string().lines() {
writeln!(out, "//{}", line);
}
let ident = if let Some(alias) = &efn.alias { alias } else { &efn.ident };
let local_name = match &efn.sig.receiver {
None => efn.ident.to_string(),
Some(receiver) => format!("{}::{}", receiver.ty, efn.ident),
None => ident.to_string(),
Some(receiver) => format!("{}::{}", receiver.ty, ident),
};
let invoke = mangle::extern_fn(&out.namespace, efn);
let indirect_call = false;
Expand Down
7 changes: 4 additions & 3 deletions macro/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ fn expand_cxx_type(namespace: &Namespace, ety: &ExternType) -> TokenStream {
}

fn expand_cxx_function_decl(namespace: &Namespace, efn: &ExternFn, types: &Types) -> TokenStream {
let ident = &efn.ident;
let ident = if let Some(alias) = &efn.alias { alias } else { &efn.ident };
let receiver = efn.receiver.iter().map(|receiver| {
let receiver_type = receiver.ty();
quote!(_: #receiver_type)
Expand Down Expand Up @@ -225,7 +225,7 @@ fn expand_cxx_function_decl(namespace: &Namespace, efn: &ExternFn, types: &Types
}

fn expand_cxx_function_shim(namespace: &Namespace, efn: &ExternFn, types: &Types) -> TokenStream {
let ident = &efn.ident;
let ident = if let Some(alias) = &efn.alias { alias } else { &efn.ident };
let doc = &efn.doc;
let decl = expand_cxx_function_decl(namespace, efn, types);
let receiver = efn.receiver.iter().map(|receiver| {
Expand Down Expand Up @@ -394,7 +394,8 @@ fn expand_function_pointer_trampoline(
let c_trampoline = mangle::c_trampoline(namespace, efn, var);
let r_trampoline = mangle::r_trampoline(namespace, efn, var);
let local_name = parse_quote!(__);
let catch_unwind_label = format!("::{}::{}", efn.ident, var);
let ident = if let Some(alias) = &efn.alias { alias } else { &efn.ident };
let catch_unwind_label = format!("::{}::{}", ident, var);
let shim = expand_rust_function_shim_impl(
sig,
types,
Expand Down
14 changes: 13 additions & 1 deletion syntax/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub struct Parser<'a> {
pub doc: Option<&'a mut Doc>,
pub derives: Option<&'a mut Vec<Derive>>,
pub repr: Option<&'a mut Option<Atom>>,
pub alias: Option<&'a mut Option<Ident>>,
}

pub(super) fn parse_doc(cx: &mut Errors, attrs: &[Attribute]) -> Doc {
Expand Down Expand Up @@ -57,8 +58,19 @@ pub(super) fn parse(cx: &mut Errors, attrs: &[Attribute], mut parser: Parser) {
}
Err(err) => return cx.push(err),
}
} else if attr.path.is_ident("alias") {
match attr.parse_args_with(|input: ParseStream| input.parse()) {
Ok(alias) => {
if let Some(a) = &mut parser.alias {
**a = Some(alias);
continue;
}
}
Err(err) => return cx.push(err),
}
} else {
return cx.error(attr, "unsupported attribute");
}
return cx.error(attr, "unsupported attribute");
Comment on lines +71 to -61
Copy link
Owner

Choose a reason for hiding this comment

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

This was correct before the PR. The change made in the PR makes well-formed attributes silently accepted anywhere where not expected, like putting a #[derive()] attribute on a function or a #[repr()] attribute on a function.

}
}

Expand Down
3 changes: 2 additions & 1 deletion syntax/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ pub(crate) fn check_all(cx: &mut Check, namespace: &Namespace, apis: &[Api]) {
check(cx, &ety.ident);
}
Api::CxxFunction(efn) | Api::RustFunction(efn) => {
check(cx, &efn.ident);
let ident = if let Some(alias) = &efn.alias { alias } else { &efn.ident };
check(cx, ident);
for arg in &efn.args {
check(cx, &arg.ident);
}
Expand Down
5 changes: 3 additions & 2 deletions syntax/mangle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ macro_rules! join {
}

pub fn extern_fn(namespace: &Namespace, efn: &ExternFn) -> Symbol {
let ident = if let Some(alias) = &efn.alias { alias } else { &efn.ident };
match &efn.receiver {
Some(receiver) => join!(namespace, CXXBRIDGE, receiver.ty, efn.ident),
None => join!(namespace, CXXBRIDGE, efn.ident),
Some(receiver) => join!(namespace, CXXBRIDGE, receiver.ty, ident),
None => join!(namespace, CXXBRIDGE, ident),
}
}

Expand Down
1 change: 1 addition & 0 deletions syntax/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ pub struct ExternFn {
pub lang: Lang,
pub doc: Doc,
pub ident: Ident,
pub alias: Option<Ident>,
pub sig: Signature,
pub semi_token: Token![;],
}
Expand Down
5 changes: 5 additions & 0 deletions syntax/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,10 @@ fn parse_extern_fn(cx: &mut Errors, foreign_fn: &ForeignItemFn, lang: Lang) -> R
let doc = attrs::parse_doc(cx, &foreign_fn.attrs);
let fn_token = foreign_fn.sig.fn_token;
let ident = foreign_fn.sig.ident.clone();
let mut alias = None;
if let Lang::Cxx = lang {
attrs::parse(cx, &foreign_fn.attrs, attrs::Parser { alias: Some(&mut alias), ..Default::default() });
}
let paren_token = foreign_fn.sig.paren_token;
let semi_token = foreign_fn.semi_token;
let api_function = match lang {
Expand All @@ -338,6 +342,7 @@ fn parse_extern_fn(cx: &mut Errors, foreign_fn: &ForeignItemFn, lang: Lang) -> R
lang,
doc,
ident,
alias,
sig: Signature {
fn_token,
receiver,
Expand Down
2 changes: 1 addition & 1 deletion syntax/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl<'a> Types<'a> {
rust.insert(ident);
}
Api::CxxFunction(efn) | Api::RustFunction(efn) => {
let ident = &efn.ident;
let ident = if let Some(alias) = &efn.alias { alias } else { &efn.ident };
if !function_names.insert((&efn.receiver, ident)) {
duplicate_name(cx, efn, ident);
}
Comment on lines +119 to 122
Copy link
Owner

Choose a reason for hiding this comment

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

As implemented in this PR, the alias of an extern Rust function is its C++ name and the alias of an extern C++ function is its Rust name. So this check isn't correct:

  • It is going to have false positives on extern Rust functions having the same C++ name, which we should allow because C++ has function overloading.
  • It is going to have false negatives on an extern C++ function with alias f colliding with an extern Rust function named f if the Rust one also has an alias.
  • It is going to have false positives on the Rust name of a C++ function being the same as the C++ name of a Rust function, which should in general have no bearing on each other.

Expand Down