Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type safety of sigc::track_obj #78

Closed
LordVolumeForm opened this issue Apr 10, 2022 · 2 comments
Closed

Type safety of sigc::track_obj #78

LordVolumeForm opened this issue Apr 10, 2022 · 2 comments

Comments

@LordVolumeForm
Copy link

LordVolumeForm commented Apr 10, 2022

libsigc++ positions itself as "The Typesafe Callback Framework for C++". However, when using sigc::track_obj, there are some serious deficiencies in type safety that can all too easily lead to slots not auto-disconnecting.

The following code illustrates the problem. It contains four attempts to create an auto-disconnecting callback:

struct X : sigc::trackable {};
struct Y {};

X x;
Y y;
sigc::signal<void()> sig;

sig.connect(sigc::track_obj([] {}, x));   // good
sig.connect(sigc::track_obj([] {}, y));   // bad but compiles
sig.connect(sigc::track_obj([] {}, &x));  // bad but compiles
sig.connect(sigc::track_obj([] {}, 42));  // bad but compiles
  1. The first is the correct way. It auto-disconnects when x is destructed.
  2. Should not be allowed, as Y is not sigc::trackable, so cannot auto-disconnect.
  3. This both looks plausible and compiles, but does not auto-disconnect.
  4. Shouldn't even compile, but does so without a hitch.

This behaviour makes sigc::track_obj extremely fragile and dangerous. Code using sigc::track_obj for auto-disconnection can start silently failing when the programmer changes a type, as shown by point 2. It is also extremely easy to use incorrectly without the slightest indication something is wrong, as shown by point 3. For safety reasons, code using sigc::track_obj should only compile if auto-disconnection will actually happen. In particular, examples 2-4 should not compile.

@kjellahl
Copy link
Contributor

Yes, sigc::track_obj() can (and should) be improved. static_assert() can probably
be used for checking that the supplied objects really are derived from sigc::trackable.

Adding restrictions to sigc::track_obj() would break API, though. Code that has
previously compiled and worked, would stop compiling. (Your examples 2, 3, 4 do
work in a way, even though they don't do exactly what was intended.)
I'd prefer to make a new function (sigc::track_obj2()?) with a static_assert(),
and deprecate sigc::track_obj().

Even with a static_assert(), sigc::track_obj() would not be completely safe.
You still have to manually tell it which trackable objects are used by the lambda expression.

@LordVolumeForm
Copy link
Author

A compile-time warning would be fine for now.

Maybe instead of track_obj2, track_obj_checked or track_obj_safe?

kjellahl added a commit that referenced this issue Apr 20, 2022
track_object() checks that the listed objects derive from sigc::trackable.
Fixes #78
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants