Skip to content

slot|signal: static_assert not using R,T... syntax #100

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.

Sign up for GitHub

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

Jump to bottom
Merged
kjellahl merged 1 commit into from
Dec 16, 2023
Merged

slot|signal: static_assert not using R,T... syntax #100

kjellahl merged 1 commit into from
Dec 16, 2023

Conversation

@db-src
Copy link
Contributor

@db-src db-src commented Nov 23, 2023

This can lead to clearer errors by explaining the user's error, not just saying they used an incomplete type (why is it incomplete?). I don't use only static_assert(false) because that's ill-formed before C++23, AFAIK, & even if it's OK in some cases I don't grok which... so hope this works

My hope is that by using the weird sizeof comparison, it bypasses the issue with static_assert(false), based on topics like this:

Tests build and run fine on g++ (Debian 13.2.0-6) 13.2.0 but I would appreciate more coverage on other compilers.

If we do this, we should do equivalent for Glib::SignalProxy in glibmm.

Fixes #86

@db-src
slot|signal: static_assert not using R,T... syntax
9ce9db2 
This can lead to clearer errors by explaining the user's error, not just
saying they used an incomplete type (why is it incomplete?). I don't use
only static_assert(false) because that's ill-formed before C++23, AFAIK,
& even if it's OK in some cases I don't grok which... so hope this works

libsigcplusplus#86
@db-src db-src mentioned this pull request Nov 23, 2023
Closed
@kjellahl
Copy link
Contributor

kjellahl commented Nov 24, 2023

It works fine with g++ 13.2.0 and clang++ 16.0.6 on Ubuntu 23.10.

@fanc999-1 If you have time, can you please check if Daniel's fix works well with MSVC.
That is, it shall build without problems and all tests shall succeed.
If for instance sigc::slot<void(int)> s1 = foo(); is changed to sigc::slot<void, int> s1 = foo();
in tests/test_slot.cc, and a similar change is made in tests/test_signal.cc,
the compiler shall generate reasonable error messages.

@kjellahl
Copy link
Contributor

kjellahl commented Dec 15, 2023

@db-src I get no reply from Chun-wei Fan. I've emailed him on two addresses
that he has used in commits. Shall I merge this PR now, or do you want it
tested with more compilers?

@db-src
Copy link
Contributor Author

db-src commented Dec 15, 2023

I would prefer to have it tested on Windows, but I don't have access to that myself currently, so if no one is around to test it at the moment, I don't think it's the end of the world if we merge it. I would expect it to be fine, but if there is an issue, someone will find it eventually and then we can fix it if needed. 😄

@kjellahl kjellahl merged commit 0e79f1a into libsigcplusplus:master Dec 16, 2023
@kjellahl
Copy link
Contributor

kjellahl commented Dec 16, 2023

All CI tests succeeded, including meson-msvc.yml.

@db-src db-src deleted the static_assert_not_old_syntax branch September 23, 2025 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Old syntax <result, arg1, arg2...> for signals/slots is commented @deprecated but not marked as such to C++ or in DISABLE_DEPRECATED

2 participants

@db-src @kjellahl