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

GUI2/Dispatcher: refactored [dis]connect_signal to use constexpr-if #5456

Merged
merged 1 commit into from Apr 1, 2022

Conversation

Vultraz
Copy link
Member

@Vultraz Vultraz commented Jan 17, 2021

Opening as a PR for comments on the design, as well as waiting until we get the MacOS requirements sorted. Not 100% this is the correct use of std::is_convertible, but it does seem to work.

@github-actions github-actions bot added the UI User interface issues, including both back-end and front-end issues. label Jan 17, 2021
@stevecotton
Copy link
Contributor

The error messages generated by both GCC and Clang are uninformative - they only hint at which .cpp file has the failing call:

In file included from src/actions/advancement.cpp:29:
In file included from src/gui/dialogs/unit_advance.hpp:16:
In file included from src/gui/dialogs/modal_dialog.hpp:18:
In file included from src/gui/core/static_registry.hpp:18:
In file included from src/gui/core/window_builder.hpp:19:
In file included from src/gui/widgets/grid.hpp:17:
In file included from src/gui/widgets/widget.hpp:17:
src/gui/core/event/dispatcher.hpp:526:4: error: static_assert failed "No matching signal queue found for event"
                        static_assert(false, "No matching signal queue found for event");
                        ^             ~~~~~
src/gui/core/event/dispatcher.hpp:570:4: error: static_assert failed "No matching signal queue found for event"
                        static_assert(false, "No matching signal queue found for event");
                        ^             ~~~~~

@stevecotton
Copy link
Contributor

Opening as a PR for comments on the design

Will this check the callback function's signature at compile time, or will std::bind() manipulate the signature to fit and leave the error to be detected at runtime?

@Vultraz
Copy link
Member Author

Vultraz commented Apr 1, 2022

For reference, this is why I couldn't use static_assert(false): https://devblogs.microsoft.com/oldnewthing/20200311-00/?p=103553

@Vultraz Vultraz merged commit 87605e7 into master Apr 1, 2022
@Vultraz Vultraz deleted the gui2-constexpr-dispatcher branch April 1, 2022 09:30
} else if constexpr(is_text_input_event(E)) {
VALIDATE_AND_ADD_TO_QUEUE(signal_text_input)
} else {
assert(false && "No matching signal queue found for event");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this replaced a compile-time check with a runtime check?

@stevecotton
Copy link
Contributor

For reference, this is why I couldn't use static_assert(false): https://devblogs.microsoft.com/oldnewthing/20200311-00/?p=103553

https://en.cppreference.com/w/cpp/language/if includes an example of how to do it as a compile-time check, using dependent_false_v.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI User interface issues, including both back-end and front-end issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants