search

dtolnay / cxx

Safe interop between Rust and C++
https://cxx.rs
Apache License 2.0
5.82k stars 330 forks source link

Fix: Use correct methods when writing toposorted structs #1320

Closed phil-opp closed 7 months ago

phil-opp commented 7 months ago

If there are other structs in toposorted_structs before strct, they were written with the methods of strct instead of their own. This commit fixes that.

This commit also fixes the check against out.types.cxx that should prevent duplicate definitions. I didn't encounter any such issues, but I still think that it makes sense to fix it.

Both issues were introduced in commit 5439fa195bbece443d261d40bf08d77012d44e58.

Example

I encountered this issue with the following bridge module:

```rust #[cxx::bridge] mod ffi { // [...] #[allow(non_camel_case_types)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[namespace = "sensor_msgs"] #[cxx_name = NavSatFix] pub struct sensor_msgs__NavSatFix { pub header: std_msgs__Header, pub status: sensor_msgs__NavSatStatus, pub latitude: f64, pub longitude: f64, pub altitude: f64, #[serde(with = "serde_big_array::BigArray")] pub position_covariance: [f64; 9usize], pub position_covariance_type: u8, } extern "Rust" { #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_UNKNOWN] pub fn const_sensor_msgs__NavSatFix_COVARIANCE_TYPE_UNKNOWN( self: &sensor_msgs__NavSatFix, ) -> u8; #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_APPROXIMATED] pub fn const_sensor_msgs__NavSatFix_COVARIANCE_TYPE_APPROXIMATED( self: &sensor_msgs__NavSatFix, ) -> u8; #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_DIAGONAL_KNOWN] pub fn const_sensor_msgs__NavSatFix_COVARIANCE_TYPE_DIAGONAL_KNOWN( self: &sensor_msgs__NavSatFix, ) -> u8; #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_KNOWN] pub fn const_sensor_msgs__NavSatFix_COVARIANCE_TYPE_KNOWN( self: &sensor_msgs__NavSatFix, ) -> u8; } #[allow(non_camel_case_types)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] #[namespace = "sensor_msgs"] #[cxx_name = NavSatStatus] pub struct sensor_msgs__NavSatStatus { pub status: i8, pub service: u16, } extern "Rust" { #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatStatus_STATUS_NO_FIX] pub fn const_sensor_msgs__NavSatStatus_STATUS_NO_FIX( self: &sensor_msgs__NavSatStatus, ) -> i8; #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatStatus_STATUS_FIX] pub fn const_sensor_msgs__NavSatStatus_STATUS_FIX( self: &sensor_msgs__NavSatStatus, ) -> i8; #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatStatus_STATUS_SBAS_FIX] pub fn const_sensor_msgs__NavSatStatus_STATUS_SBAS_FIX( self: &sensor_msgs__NavSatStatus, ) -> i8; #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatStatus_STATUS_GBAS_FIX] pub fn const_sensor_msgs__NavSatStatus_STATUS_GBAS_FIX( self: &sensor_msgs__NavSatStatus, ) -> i8; #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatStatus_SERVICE_GPS] pub fn const_sensor_msgs__NavSatStatus_SERVICE_GPS( self: &sensor_msgs__NavSatStatus, ) -> u16; #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatStatus_SERVICE_GLONASS] pub fn const_sensor_msgs__NavSatStatus_SERVICE_GLONASS( self: &sensor_msgs__NavSatStatus, ) -> u16; #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatStatus_SERVICE_COMPASS] pub fn const_sensor_msgs__NavSatStatus_SERVICE_COMPASS( self: &sensor_msgs__NavSatStatus, ) -> u16; #[namespace = "sensor_msgs"] #[cxx_name = const_sensor_msgs_NavSatStatus_SERVICE_GALILEO] pub fn const_sensor_msgs__NavSatStatus_SERVICE_GALILEO( self: &sensor_msgs__NavSatStatus, ) -> u16; } // [...] } ```

The code generated by cxx was:


#ifndef CXXBRIDGE1_STRUCT_sensor_msgs$NavSatStatus
#define CXXBRIDGE1_STRUCT_sensor_msgs$NavSatStatus
struct NavSatStatus final {
  ::std::int8_t status;
  ::std::uint16_t service;

  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_UNKNOWN() const noexcept;
  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_APPROXIMATED() const noexcept;
  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_DIAGONAL_KNOWN() const noexcept;
  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_KNOWN() const noexcept;
  bool operator==(NavSatStatus const &) const noexcept;
  bool operator!=(NavSatStatus const &) const noexcept;
  using IsRelocatable = ::std::true_type;
};
#endif // CXXBRIDGE1_STRUCT_sensor_msgs$NavSatStatus

#ifndef CXXBRIDGE1_STRUCT_sensor_msgs$NavSatFix
#define CXXBRIDGE1_STRUCT_sensor_msgs$NavSatFix
struct NavSatFix final {
  ::std_msgs::Header header;
  ::sensor_msgs::NavSatStatus status;
  double latitude;
  double longitude;
  double altitude;
  ::std::array<double, 9> position_covariance;
  ::std::uint8_t position_covariance_type;

  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_UNKNOWN() const noexcept;
  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_APPROXIMATED() const noexcept;
  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_DIAGONAL_KNOWN() const noexcept;
  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_KNOWN() const noexcept;
  bool operator==(NavSatFix const &) const noexcept;
  bool operator!=(NavSatFix const &) const noexcept;
  using IsRelocatable = ::std::true_type;
};
#endif // CXXBRIDGE1_STRUCT_sensor_msgs$NavSatFix

Note how both the NavSatStatus and NavSatFix structs are assigned the NavSatFix methods. This bug leads to a compile error in the generated .cc file:

bindings.cc:37786:29: error: out-of-line definition of 'const_sensor_msgs_NavSatStatus_STATUS_NO_FIX' does not match any declaration in 'sensor_msgs::NavSatStatus'
::std::int8_t NavSatStatus::const_sensor_msgs_NavSatStatus_STATUS_NO_FIX() const noexcept {
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

After applying this PR, the generated code is correct:

#ifndef CXXBRIDGE1_STRUCT_sensor_msgs$NavSatStatus
#define CXXBRIDGE1_STRUCT_sensor_msgs$NavSatStatus
struct NavSatStatus final {
  ::std::int8_t status;
  ::std::uint16_t service;

  ::std::int8_t const_sensor_msgs_NavSatStatus_STATUS_NO_FIX() const noexcept;
  ::std::int8_t const_sensor_msgs_NavSatStatus_STATUS_FIX() const noexcept;
  ::std::int8_t const_sensor_msgs_NavSatStatus_STATUS_SBAS_FIX() const noexcept;
  ::std::int8_t const_sensor_msgs_NavSatStatus_STATUS_GBAS_FIX() const noexcept;
  ::std::uint16_t const_sensor_msgs_NavSatStatus_SERVICE_GPS() const noexcept;
  ::std::uint16_t const_sensor_msgs_NavSatStatus_SERVICE_GLONASS() const noexcept;
  ::std::uint16_t const_sensor_msgs_NavSatStatus_SERVICE_COMPASS() const noexcept;
  ::std::uint16_t const_sensor_msgs_NavSatStatus_SERVICE_GALILEO() const noexcept;
  bool operator==(NavSatStatus const &) const noexcept;
  bool operator!=(NavSatStatus const &) const noexcept;
  using IsRelocatable = ::std::true_type;
};
#endif // CXXBRIDGE1_STRUCT_sensor_msgs$NavSatStatus

#ifndef CXXBRIDGE1_STRUCT_sensor_msgs$NavSatFix
#define CXXBRIDGE1_STRUCT_sensor_msgs$NavSatFix
struct NavSatFix final {
  ::std_msgs::Header header;
  ::sensor_msgs::NavSatStatus status;
  double latitude;
  double longitude;
  double altitude;
  ::std::array<double, 9> position_covariance;
  ::std::uint8_t position_covariance_type;

  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_UNKNOWN() const noexcept;
  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_APPROXIMATED() const noexcept;
  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_DIAGONAL_KNOWN() const noexcept;
  ::std::uint8_t const_sensor_msgs_NavSatFix_COVARIANCE_TYPE_KNOWN() const noexcept;
  bool operator==(NavSatFix const &) const noexcept;
  bool operator!=(NavSatFix const &) const noexcept;
  using IsRelocatable = ::std::true_type;
};
#endif // CXXBRIDGE1_STRUCT_sensor_msgs$NavSatFix
dtolnay commented 7 months ago

Published in cxx 1.0.118.

phil-opp commented 7 months ago

Thanks for the quick release!