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

Make UserData pointer-packing actually sound #223

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
13 changes: 13 additions & 0 deletions physx-sys/pxbind/src/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,11 @@ pub enum Builtin {
Mat33,
Mat34,
Mat44,
// on the C++ side, `void*`, but we want to be able to pack arbitrary data into the space
// available so we treat it as a separate type so we can do that properly in the Rust side
UserData,
// same as above but `const void*
ConstUserData,
}

impl Builtin {
Expand Down Expand Up @@ -758,6 +763,8 @@ impl Builtin {
Self::Mat34V => "glam::Affine3A",
Self::Mat34 => "Affine",
Self::Mat44V | Self::Mat44 => "PxMat44",
Self::UserData => "UserData",
Self::ConstUserData => "ConstUserData",
}
}

Expand Down Expand Up @@ -793,6 +800,8 @@ impl Builtin {
Self::Mat34 => "physx_Mat34_Pod",
Self::Mat44V => "physx_Mat44V_Pod",
Self::Mat44 => "physx_PxMat44_Pod",
Self::UserData => "void*",
Self::ConstUserData => "void const*",
}
}

Expand Down Expand Up @@ -828,6 +837,8 @@ impl Builtin {
Self::Mat34 => "physx::PxMat34",
Self::Mat44V => "physx::PxMat44V",
Self::Mat44 => "physx::PxMat44",
Self::UserData => "void*",
Self::ConstUserData => "void const*",
}
}

Expand All @@ -847,6 +858,8 @@ impl Builtin {
| Self::UInt
| Self::Long
| Self::ULong
| Self::UserData
| Self::ConstUserData
)
}
}
Expand Down
42 changes: 32 additions & 10 deletions physx-sys/pxbind/src/consumer/functions.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{Comment, Item, Method, QualType, TemplateArg};
use super::{Builtin, Comment, Item, Method, QualType, TemplateArg};
use crate::Node;
use anyhow::Context as _;
use std::borrow::Cow;
Expand Down Expand Up @@ -197,14 +197,22 @@ impl<'ast> super::AstConsumer<'ast> {
.name
.as_deref()
.map_or_else(|| format!("anon_param{i}").into(), Cow::Borrowed);
let kind = self
.parse_type(&param.kind, template_types)
.with_context(|| {
format!(
"failed to parse parameter '{pname} ({})' for function '{name}'",
param.kind.qual_type,
)
})?;

let kind = if param.name.as_ref().map_or(false, |name| name == "userData") {
if param.kind.qual_type.contains("const") {
QualType::Builtin(Builtin::ConstUserData)
} else {
QualType::Builtin(Builtin::UserData)
}
} else {
self.parse_type(&param.kind, template_types)
.with_context(|| {
format!(
"failed to parse parameter '{pname} ({})' for function '{name}'",
param.kind.qual_type,
)
})?
};

func.params.push(Param { name: pname, kind });
}
Expand All @@ -229,7 +237,21 @@ impl<'ast> super::AstConsumer<'ast> {
.with_context(|| format!("function signature for '{name}' doesn't have a '('"))?;

let ret = sig[..open_ind].trim();
if ret != "void" {
if name.contains("getUserData") {
if ret == "void*" || ret == "void *" {
func.ret = Some(QualType::Builtin(Builtin::UserData));
} else {
log::warn!(
"found function with name containing getUserData that had unexpected return type: {ret}"
);
if ret != "void" {
func.ret =
Some(self.parse_type(ret, template_types).with_context(|| {
format!("failed to parse return type for '{name}'")
})?);
}
}
} else if ret != "void" {
func.ret = Some(
self.parse_type(ret, template_types)
.with_context(|| format!("failed to parse return type for '{name}'"))?,
Expand Down
14 changes: 11 additions & 3 deletions physx-sys/pxbind/src/consumer/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -713,9 +713,17 @@ impl<'ast> super::AstConsumer<'ast> {
continue;
}

let kind = self
.parse_type(kind, template_types)
.with_context(|| format!("failed to parse type for {rname}::{name}"))?;
let kind = if name == "userData" || name == "mUserData" {
if kind.qual_type.contains("const") {
QualType::Builtin(Builtin::ConstUserData)
} else {
QualType::Builtin(Builtin::UserData)
}
} else {
self.parse_type(kind, template_types).with_context(|| {
format!("failed to parse type for {rname}::{name}")
})?
};

// if matches!(&kind, QualType::FunctionPointer) {
// continue;
Expand Down
Loading