CI (up to test_rclcpp rclcpp, fastrtps only)

• Linux
• Linux-aarch64
• macOS
• Windows

The current change is not ABI compatible and can therefore not be backported to Dashing as-is.

@ivanpauno was this selected for a specific use case to be backported? The simple workaround seems to be to just pass the default value explicitly.

@esteve @ivanpauno If either of you still thinks this should be backported can you please create a custom PR targeting the dashing branch which adds a new constructor without the options argument which calls the existing constructor with the explicit default value.

The current change is not ABI compatible and can therefore not be backported to Dashing as-is.

@dirk-thomas How does this break ABI? It looks like the constructor has the same number of arguments with the same types as before.

I think that usually defaults are implemented as temporary objects created at the point of call, and it calls the same symbol when using the default than when passing the parameter explicitly.
Compilers may create two symbols in this case, one for the method taking the default and one for the non default.

In the first case this would be ABI compatible, in the second not.
I think that most compilers do the first thing, but I can't confirm that.

For me, it's okay not doing a backport.

I couldn't come up with a reference. With g++ I can confirm that the symbol stays the same so for that compiler the change is ABI compatible.

In the first case this would be ABI compatible, in the second not.

The second case would add a new (non-virtual) constructor with one argument less. That change should be ABI compatible too. I will take back my objection about the ABI compatibility then.

The other question remains though: why should this be backported?

The other question remains though: why should this be backported?

There's not good reason, so I think not doing the backport is fine.
I'm deleting it from the project board.

@dirk-thomas The other question remains though: why should this be backported?

I don't really see why not, it'd make the lifecycle API consistent with the node API, it's not breaking the ABI and also fixes the documentation.

Since there is another backport for this repo I don't mind to also pick this one, see #801.