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

Add failing test for extern_rust_function #1166

Merged
merged 1 commit into from
Oct 20, 2022
Merged

Add failing test for extern_rust_function #1166

merged 1 commit into from
Oct 20, 2022

Conversation

Ravenslofty
Copy link
Contributor

Per the Contributing section, the best way to submit a bug is to submit a PR of a failing integration test, so, here's a minimal case from some code I've been trying to write, where a Rust function receives a C++ context, and cxx rejects the generated code with an unsupported type error (lacking a span, which makes it unhelpful).

@adetaylor
Copy link
Collaborator

Thanks very much!

If I had to guess, I'd say the problem is the &mut a in pub fn called_from_cpp(a: &mut ffi::a) {}. What happens if you switch that to Pin<&mut a>? However - it absolutely wouldn't surprise me if that doesn't fix it, and even if it does, the diagnostics should be better, so I'll poke around at what happens when running your test case.

@Ravenslofty
Copy link
Contributor Author

Indeed, Pin<&mut ffi::a> makes no difference, and I thought it was a little simpler without the Pin, even if that would be more correct. I can change the test case to use Pin if you prefer?

adetaylor added a commit that referenced this pull request Oct 17, 2022
adetaylor added a commit that referenced this pull request Oct 20, 2022
Previously extern_rust_function support was highly basic and
didn't usually work at all. This change improevs matters:

* it allows us to emit errors with miette diagnostics from
  this part of the codebase, giving spans in case of trouble
* it validates that the parameters to such a function are
  valid to be used by cxx, as is the return type
* it detects types used as parameters and return type and
  ensures they are not garbage collected elsewhere in autocxx
* it adds a code example.
* we remove a field from RustFun which we didn't actually need
  and replace it with a simple Boolean.

Builds on test case from #1166 (thanks!) and there's a bit of
discussion in #1167.
adetaylor added a commit that referenced this pull request Oct 20, 2022
Previously extern_rust_function support was highly basic and
didn't usually work at all. This change improevs matters:

* it allows us to emit errors with miette diagnostics from
  this part of the codebase, giving spans in case of trouble
* it validates that the parameters to such a function are
  valid to be used by cxx, as is the return type
* it detects types used as parameters and return type and
  ensures they are not garbage collected elsewhere in autocxx
* it adds a code example.
* we remove a field from RustFun which we didn't actually need
  and replace it with a simple Boolean.

Builds on test case from #1166 (thanks!) and there's a bit of
discussion in #1167.
@adetaylor adetaylor merged commit 93657b8 into google:main Oct 20, 2022
@adetaylor
Copy link
Collaborator

Landed in #1168 (thanks!)

@Ravenslofty Ravenslofty deleted the extern_rust_fn branch October 21, 2022 08:48
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