Fix: Use correct methods when writing toposorted structs #1320

phil-opp · 2024-02-28T10:55:16Z

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 5439fa1.

Example

I encountered this issue with the following bridge module:

#[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

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 5439fa1.

dtolnay

Thank you!

dtolnay · 2024-02-28T18:49:37Z

Published in cxx 1.0.118.

phil-opp · 2024-02-29T10:33:57Z

Thanks for the quick release!

phil-opp mentioned this pull request Feb 28, 2024

Expose ROS2 constants in generated bindings (Rust and C++) dora-rs/dora#428

Merged

dtolnay approved these changes Feb 28, 2024

View reviewed changes

dtolnay merged commit c348175 into dtolnay:master Feb 28, 2024
15 checks passed

phil-opp deleted the use-correct-methods branch February 29, 2024 10:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Use correct methods when writing toposorted structs #1320

Fix: Use correct methods when writing toposorted structs #1320

phil-opp commented Feb 28, 2024 •

edited

Loading

dtolnay left a comment

dtolnay commented Feb 28, 2024

phil-opp commented Feb 29, 2024

Fix: Use correct methods when writing toposorted structs #1320

Fix: Use correct methods when writing toposorted structs #1320

Conversation

phil-opp commented Feb 28, 2024 • edited Loading

Example

dtolnay left a comment

Choose a reason for hiding this comment

dtolnay commented Feb 28, 2024

phil-opp commented Feb 29, 2024

phil-opp commented Feb 28, 2024 •

edited

Loading