-
Notifications
You must be signed in to change notification settings - Fork 164
Closed
Description
This new warning is causing CI to fail
warning: `extern` block uses type `u128`, which is not FFI-safe
--> /home/sam/rust_ws/src/ros2_rust/rclrs_tests/target/debug/build/rclrs-8555e18c0c9bb180/out/rcl_bindings_generated.rs:6343:31
|
6343 | dynamic_type_builder: *mut rosidl_dynamic_typesupport_dynamic_type_builder_t,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
|
= note: 128-bit integers don't currently have a known stable ABI
Seems to be coming from auto-generated idl code. Investigate and hopefully resolve
Metadata
Metadata
Assignees
Labels
No labels
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
esteve commentedon Jul 12, 2023
It's interesting that this only happens with
rolling
. As I see it, we have two options:dyn_msg
) from theclippy
checku128
(viaimproper-ctypes
) from theclippy
checkI believe that disabling the check from
clippy
should be fine in our case, as we're not exposing this API.@maspe36 what do you think?
maspe36 commentedon Jul 13, 2023
This is also happening with
iron
in my PR to add support for it. I'm trying to hunt down the root cause and depending on that it may indeed make sense to disable the warning or dynamic message feature from clippy.The first build I found where this was failing the rolling build #974.
Current theory is that this is related to the version uptick in rosidl_generator_c from 3.3.1 to 4.1.1 as that is a major version change between builds #974 and #973. I'll keep digging
maspe36 commentedon Jul 13, 2023
Looks like this is cause by the new Runtime Interface Reflection feature (#215) in the rmw_implementation package.
However, the original type that is ultimately by translated by bindgen(?) to a u128 is defined here in the newly stabilized in Iron, rosidl_dynamic_typesupport package.
It seems like the
dyn_msg
feature is a custom feature we use and frankly I don't know much about cargo features. I think the best course would be to disable clippy lints for the auto generated bindgen code.But... Isn't that what we're doing here? Safe to say I'm a bit lost
esteve commentedon Jul 13, 2023
@maspe36 adding
#![allow(improper_ctypes)]
should suppress that warning (see https://github.com/CCExtractor/rusty_ffmpeg/pull/13/files). I'll build locally with that change and see if it works.improper_ctypes_definitions
lint on our generated rcl bindings #341maspe36 commentedon Apr 16, 2024
Nothing for us to do at this time, but there has been some movement upstream which will eventually trickle down to us
rust-lang/rust#54341
oriongonza commentedon May 13, 2024
u128 is already FFI safe: https://blog.rust-lang.org/2024/03/30/i128-layout-update.html
maspe36 commentedon May 13, 2024
Yup, this is now part of stable Rust as of May 2nd. To take advantage we will need to upgrade from 1.74 to 1.78.
Issue for this upgrade created here #398