autocxx icon indicating copy to clipboard operation
autocxx copied to clipboard

`noexcept` not supported and can mess up subclasses

Open SteffenLindner opened this issue 11 months ago • 3 comments

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();

};

SteffenLindner avatar Feb 10 '25 12:02 SteffenLindner

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

adetaylor avatar Feb 10 '25 13:02 adetaylor

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

SteffenLindner avatar Feb 17 '25 08:02 SteffenLindner

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, https://github.com/rust-lang/rust-bindgen/pull/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 avatar Feb 24 '25 16:02 adetaylor