Remove unnecessary extern "C"

[rotu]

Signed-off-by: Dan Rose [email protected]

[hidmic]

CI up to rcl_logging_noop, rcl_logging_spdlog, rcl and rclcpp:

• Linux

[nuclearsandwich]

CI up to rcl_logging_noop, rcl_logging_spdlog, rcl and rclcpp:

* Linux

There was a mistake in the above repos file re-triggered as

• Linux

[clalancette]

Yeah, I wondered about that, and I have to admit that I cargo-culted it when I did rcl_logging_spdlog.cpp. As long as CI comes back green (including tests in at least one package that uses this code), I'm happy.

[clalancette]

Actually, I think I'm totally wrong.

The reason we need the extern "C" is that we are filling in a C API, so we need to use C symbol names, not C++ symbol names. So I'm pretty sure this isn't going to work as-is.

[rotu]

The language linkage is only needed on the declaration, not the definition. That’s why I don’t think it’s necessary here.

[clalancette]

The language linkage is only needed on the declaration, not the definition. That’s why I don’t think it’s necessary here.

Ah, good point. I didn't realize that.

Nothing to do with this PR, but this only works because the APIs here define their own "logging_interface" header files. The one that is located at https://github.com/ros2/rcl/blob/51886399ddc67e241521bad81975d365a4e330df/rcl/include/rcl/logging_external_interface.h doesn't have the proper extern "C". One of these days I'll clean it up so that we can just all use the same header file, but today is not that day.

[rotu]

You're right. That's... really fragile...

[rotu]

I don't feel comfortable merging this until the the interface declarations are unified.

[clalancette]

I don't feel comfortable merging this until the the interface declarations are unified.

That's fine, we can clean that up and then think about merging this.

[clalancette]

Just as an FYI, the reason that this isn't trivial to fix is that we can't have the rcl_logging* package depend on rcl, as that would introduce a circular dependency. Maybe the right thing to do is to move the logging_external_interface.h header out of rcl and have it live in this repository as an rcl_logging_interface package, but we still have to deal with the fact that logging_external_interface.h uses some types from rcl.

[rotu]

Just as an FYI, the reason that this isn't trivial to fix is that we can't have the rcl_logging* package depend on rcl, as that would introduce a circular dependency. Maybe the right thing to do is to move the logging_external_interface.h header out of rcl and have it live in this repository as an rcl_logging_interface package, but we still have to deal with the fact that logging_external_interface.h uses some types from rcl.

I think that's exactly what should be done. Here are the dependencies on rcl. I don't know what belongs in the different namespaces rcl/rcutils/rmw, but I'm assuming the following all actually belong in rcutils:

RCL_WARN_UNUSED: already exists as RCUTILS_WARN_UNUSED. I think we should just deprecate RCL_WARN_UNUSED
rcl_ret_t typedef'd to rmw_ret_t typedef'd to int32_t

[clalancette]

I'm closing this out, because the changes here were covered by #41. Please feel free to reopen if you think this is in error.