Skip to content

EvenQueue: fix template functions passing UserAllocatedEvent<...> as argument. #11391

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

Merged
merged 1 commit into from
Sep 3, 2019
Merged

EvenQueue: fix template functions passing UserAllocatedEvent<...> as argument. #11391

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions events/EventQueue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,8 @@ class EventQueue : private mbed::NonCopyable<EventQueue> {
* @return true if event was successfully cancelled
* false if event was not cancelled (invalid queue or executing already begun)
*/
template<typename... Args>
bool cancel(UserAllocatedEvent<Args...> *event)
template<typename F, typename A>
bool cancel(UserAllocatedEvent<F, A> *event)
Copy link
Contributor

@kjbracey kjbracey Sep 3, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Original code wasn't technically wrong, but I can see why ARMC5 might have got confused.

We're not actually instantiating anything here. UserAllocatedEvent takes two template arguments, and we're declaring something that could potentially give it variable.

There should only be a parameter mismatch if this was instantiated with Args... having anything other than 2 arguments. And that would never happen by function parameter deduction (there are no UserAllocatedEvents with anything other than 2 arguments to pass). It should only barf if explicitly called as cancel<int,int,int>(nullptr).

Presumably ARMC5 has some logic that crosschecks template arguments and parameters when a template is declared, and that's activating here, and thinking "variadic" doesn't match "2". That check may have been reasonable before variadic arguments. It catches this:

template <typename A, typename B>
class Foo;

template <typename A>
bool cancel(Foo<A> *event);

I don't actually know whether that is legal; and if it isn't, is a compiler is required to diagnose it if cancel is never instantiated?

Anyway, this is an example of the sort of ARMC5 glitch we're going to keep hitting. Its C++11 implementation is incomplete, and buggy. We'll have to keep working around it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Anyway, this is an example of the sort of ARMC5 glitch we're going to keep hitting. Its C++11 implementation is incomplete, and buggy. We'll have to keep working around it.

😭 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I have section "11.13 C++11 supported features" from the ARM C 5.06 manual printed to 3 sheets of A4 and pinned to the side of my cubicle.

Recommended reading for anyone thinking of pushing their C++11 luck.

{
if (event->_equeue != &_equeue) {
return false;
Expand Down Expand Up @@ -213,8 +213,8 @@ class EventQueue : private mbed::NonCopyable<EventQueue> {
* Undefined if id is invalid.
*
*/
template<typename... Args>
int time_left(UserAllocatedEvent<Args...> *event)
template<typename F, typename A>
int time_left(UserAllocatedEvent<F, A> *event)
{
if (event && event->_equeue != &_equeue) {
return -1;
Expand Down