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
merged 1 commit into from
Dec 16, 2023
Merged

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

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

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

@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

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

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