autocxx

autocxx copied to clipboard

Minimal Working Example

(I think it's minimal. I'll try to add a pull request with a new test in due course.)

Test.h

#pragma once

namespace test {
  class A;
  class B;
}

namespace test {

class A {
public:
  A() {}
  ~A() {}
  // the following line makes Test2 Opaque, and deprives it of a make_unique()
  // comment next line, uncomment the one after to obtain a make_unique() for Test2
  virtual int b() = 0;
  // int b() { return 2; }
};
  
class B: public A {
public:
  B() {}
  ~B() {}
  int b() { return 3; }
};

}

build.rs

fn main() {
    let path = std::path::PathBuf::from("src");
    let mut b = autocxx_build::Builder::new("src/main.rs", &[&path]).expect_build();
    b.flag_if_supported("-std=c++14").compile("autocxx-demo");
    println!("cargo:rerun-if-changed=src/main.rs");
}

main.rs

use autocxx::prelude::*;

include_cpp! {
    #include "Test.h"
    safety!(unsafe)
    generate!("test::B")
}

fn main() {
    // I'd like to initialize a B here, but I can't
}

Expected Behavior

The generated struct B has a make_unique implementation.

Actual Behavior

The generated struct B has a Drop implementation but no make_unique. It is not clear how I could instantiate a variable of type B.

Steps to Reproduce the Problem

See above

Specifications

• Version: 0.16.0 (or later if there's a later)
• Platform: Windows

Thanks. Yep, a pull request with a test case would be greatly appreciated and will likely yield a quicker fix. Nevertheless thanks for the minimal test case.

I've added an integration test; see pull request #775 .

Thanks for the test!

OK, an initial diagnosis by running your test and looking at the diagnostics. I'd argue that we have three separate bugs here.

• autocxx currently requires all base classes to be specified on the allowlist as generate! items. Bug 1 is that we failed to document that requirement.
• Bug 2 is that this requirement was based on a flawed assumption, which is that we wouldn't be told about base classes by bindgen unless they were allowlisted. It does rather look like bindgen tells us about base classes automatically, so it's quite possible that this requirement can simply be removed.
• Bug 3, however, is that it still doesn't work even if both A and B are given on the allowlist. This is because we don't currently detect subclasses which have concrete implementations of pure virtual base class functions. This is just an omission from our feature set.

I plan to confirm my understanding regarding Bug 2, and if so, remove this code that relates to allowlisting base classes. Obviously if all's well, that renders the documentation change ("bug 1") unnecessary.

If all's well there, I'll go ahead and tackle Bug 3 which shouldn't be drastically hard.

It would be great if you can accept the CLA so I can land your test. Thanks again for the report!

I had to create a new pull request in order to accept the CLA. Sorry for the inconvenience, but the new one allowed me to accept the CLA and is otherwise identical.

No problem at all, sorry for the inconvenience of the CLA.

I have fixed "Bug 2" (which renders "Bug 1" irrelevant). This is already a usability improvement for autocxx users, so thanks for that.

Bug 3 will be a little more involved as we'll have to track which pure virtual functions have been overridden and therefore the net 'abstract'ness of each type. If anyone reading this wants to have a go at it, the code is in autocxx-engine/src/conversion/analysis/abstract_types.rs - the loop around line 57 needs to be much more sophisticated.