google / autocxx

Tool for safe ergonomic Rust/C++ interop driven from existing C++ headers
https://docs.rs/autocxx
Apache License 2.0
2.24k stars 144 forks source link

autocxx does not generate constructors #1155

Closed bobbbay closed 1 year ago

bobbbay commented 2 years ago

Expected Behavior

Upon providing a class, autocxx should create a constructor method (or two, in this case).

Actual Behavior

It does not. autocxx succesfully generates other methods (yay!), but I have no way to create the desired structure type itself.

Steps to Reproduce the Problem

// Header file
#pragma once

#include <stdint.h>

#include <hal/SimDevice.h>
#include <wpi/sendable/Sendable.h>
#include <wpi/sendable/SendableHelper.h>

#include "frc/SPI.h"
#include "frc/interfaces/Gyro.h"

namespace frc {

class ADXRS450_Gyro : public Gyro {
 public:
  /**
   * %Gyro constructor on onboard CS0.
   */
  ADXRS450_Gyro();

  /**
   * %Gyro constructor on the specified SPI port.
   *
   * @param port The SPI port the gyro is attached to.
   */
  explicit ADXRS450_Gyro(SPI::Port port);

  ~ADXRS450_Gyro() override = default;

  ADXRS450_Gyro(ADXRS450_Gyro&&) = default;
  ADXRS450_Gyro& operator=(ADXRS450_Gyro&&) = default;

  // more methods here - these are generated successfully.
};

}  // namespace frc
// C++ file
#include "x/ADXRS450_Gyro.h"

using namespace frc;

ADXRS450_Gyro::ADXRS450_Gyro() : ADXRS450_Gyro(SPI::kOnboardCS0) {}

ADXRS450_Gyro::ADXRS450_Gyro(SPI::Port port)
    : m_spi(port), m_port(port), m_simDevice("Gyro:ADXRS450", port) {
  // more construction here...
}

// ... more methods here - these are generated successfully.
// build.rs
    let path = PathBuf::from(format!("{out_dir}/raw/"));
    let mut b = autocxx_build::Builder::new("src/io/gyroscopes.rs", &[&path])
        .extra_clang_args(&["-std=c++17", "-stdlib=libc++"])
        .build()?;
    b.flag_if_supported("-std=c++17")
        .flag_if_supported("-stdlib=libc++")
        .compile("x-gyros");
// gyroscopes.rs
    use autocxx::prelude::*;

    include_cpp! {
        #include "frc/ADXRS450_Gyro.h"
        safety!(unsafe_ffi)
        generate_pod!("frc::ADXRS450_Gyro")
    }

Specifications

adetaylor commented 2 years ago

Thanks for the report. Constructors should normally be generated just as you expect. I wonder if the problem is that you're using generate_pod! rather than generate! - I'd be curious to know if that alters the behavior. I would still consider it a bug - we should generate those constructors either way, but generate_pod! has been much less extensively tested.

Either way, would you mind raising a PR with a test case per https://google.github.io/autocxx/contributing.html#reporting-bugs? Thanks.

bobbbay commented 2 years ago

Unfortunately, generate has the same effect.

Strangely enough, I can't reproduce this, and other classes in the same library build successfully with a new and new1 function. I will try to reproduce this and add an integration test accordingly, thanks!

GoldsteinE commented 2 years ago

I’m having the same issue. I can’t share the code though, since it’s proprietary, but I can confirm that this reproduces on other code and other machine.

GoldsteinE commented 2 years ago

All subclasses of abstract classes seem to be considered abstract, even if they override every virtual method.

adetaylor commented 2 years ago

All subclasses of abstract classes seem to be considered abstract, even if they override every virtual method.

Yep, that's #744. Pull requests welcome :)

GoldsteinE commented 2 years ago

@adetaylor Why autocxx doesn’t generate non-default constructors for abstract types? If a type has a non-default constructor, it seems to automatically not be abstract, so generating of non-default constructors should always be ok, as far as I understand.

GoldsteinE commented 2 years ago

Oh, I was wrong and abstract type can have a non-default constructor, but it’ll fail at link time. Weird.

adetaylor commented 2 years ago

Oh, I was wrong and abstract type can have a non-default constructor, but it’ll fail at link time. Weird.

Could you raise a pull request with a test case for that? integration-tests/src/integration_test.rs

adetaylor commented 1 year ago

Some progress has been made on better identifying abstract types. There's therefore a chance that this has improved. In any case, this report is not currently actionable for me - I need a small minimized instance of some kind (whether a test case, a standalone repo or whatever), so I'm going to close this.