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

noexcept not supported and can mess up subclasses #1435

Open
SteffenLindner opened this issue Feb 10, 2025 · 3 comments
Open

noexcept not supported and can mess up subclasses #1435

SteffenLindner opened this issue Feb 10, 2025 · 3 comments
Labels
cpp-feature C++ feature not yet supported

Comments

@SteffenLindner
Copy link

SteffenLindner commented Feb 10, 2025

Describe the bug
When defining a virtual noexcept method in a C++ class that is derived using #[subclass(superclass("MyCppClass"))], the generated wrapper methods do not include the noexcept statement, leading to a compiler error due to looser exception statement.

To Reproduce

// src/code.hpp
#pragma once
class SuperClass
{
public:
    virtual void doSomething() const noexcept = 0;
    virtual ~SuperClass() {};
};
// build.rs
fn main() -> miette::Result<()> {
    let mut b = autocxx_build::Builder::new("src/main.rs", &["./src"])
        .extra_clang_args(&["-std=c++17"])
        .auto_allowlist(true)
        .build()?;

    b.flag_if_supported("-std=c++17").compile("noexcept-test");
    println!("cargo:rerun-if-changed=src/main.rs");
    println!("cargo:rerun-if-changed=src/code.hpp");
    Ok(())
}
// src/main.rs
use autocxx::prelude::*;
use autocxx::subclass::prelude::*;
use ffi::SuperClass_methods;

include_cpp! {
    #include "code.hpp"
    safety!(unsafe) // unsafety policy; see docs
}

#[subclass(superclass("SuperClass"))]
#[derive(Default)]
pub struct DerivedStruct {}

impl ffi::SuperClass_methods for DerivedStruct {
    fn doSomething(&self) {
        println!("Do something");
    }
}

fn main() {
    let t = DerivedStruct::default_rust_owned();
    t.borrow().doSomething();
}

Expected behavior
Print statement is executed of doSomething call, which is done if the noexcept is removed from the virtual method.

Additional context
Error:

cargo:warning=In file included from /home/XXX/autocxx-bugs/noexcept/target/debug/build/noexcept-568b2dad4acfbf95/out/autocxx-build-dir/cxx/gen0.cxx:2: cargo:warning=/home/XXX/autocxx-bugs/noexcept/target/debug/build/noexcept-568b2dad4acfbf95/out/autocxx-build-dir/include/autocxxgen_ffi.h:41:6: error: looser exception specification on overriding virtual function 'virtual void DerivedStructCpp::doSomething() const' cargo:warning= 41 | void doSomething() const; cargo:warning= | ^~~~~~~~~~~ cargo:warning=In file included from /home/XXX/autocxx-bugs/noexcept/target/debug/build/noexcept-568b2dad4acfbf95/out/autocxx-build-dir/cxx/gen0.cxx:1: cargo:warning=./src/code.hpp:5:18: note: overridden function is 'virtual void SuperClass::doSomething() const noexcept' cargo:warning= 5 | virtual void doSomething() const noexcept = 0;

I belive it should be rather straightforward to also pass the noexcept to the generated code by autocxx / cxx.

In the generated C++ code, the noexcept is missing:

// XX//autocxxgen_ffi.h
struct DerivedStructHolder;
class DerivedStructCpp : public SuperClass
{
public:
inline  DerivedStructCpp(rust::Box<DerivedStructHolder> arg0) : obs(std::move(arg0)) {  }
void doSomething() const;
const SuperClass& As_SuperClass() const { return *this; }
SuperClass& As_SuperClass_mut() { return *this; }
void DerivedStructCpp_remove_ownership() const;
private:rust::Box<DerivedStructHolder> obs;
void really_remove_ownership();

};
@adetaylor
Copy link
Collaborator

Thanks for the report - please could you submit a PR with a test case to integration_tests.rs.

SteffenLindner added a commit to SteffenLindner/autocxx-noexcept that referenced this issue Feb 10, 2025
@SteffenLindner
Copy link
Author

Sure, I added a failing test case in #1436.

@adetaylor
Copy link
Collaborator

Thanks for that - it helped me get to looking at this a lot sooner than I otherwise would have done.

Here's what it would take to fix this. I'm not going to do this any time soon, but perhaps this will help others who want to do it sooner.

The first (main) problem is that bindgen does not inform autocxx about the noexcept qualifier. autocxx literally does not know about it. This is not really bindgen's fault - it isn't designed with the assumption that downstream postprocessors will generate extra C++ code based on the information it emits. #124 is the list of things where we ask bindgen to give us more information. There's a long list of pull requests there which we're trying to upstream. Specifically, rust-lang/rust-bindgen#3146 will feed us much more information about C++ methods. We'd want to do something similar, but for functions and methods, to inform us about the noexcept qualifier. This in turn assumes that bindgen can get that information out of clang_sys.

We would then presumably get this information in a ParseCallback and we'd simply want to store it in the Api::Fn. It would then be straightforward to add this when generating extra such functions for subclasses, within the codegen_cpp area.

Overall the thing to do is to wait a couple of weeks for the PRs in #124 to be reviewed, and for us to move closer to an upstream bindgen, and then see how easy it would be to add this information.

@adetaylor adetaylor changed the title noexcept not passed to derived wrapper class noexcept not supported and can mess up subclasses Feb 24, 2025
@adetaylor adetaylor added the cpp-feature C++ feature not yet supported label Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp-feature C++ feature not yet supported
Projects
None yet
Development

No branches or pull requests

2 participants