Page MenuHomePhabricator

Calling convention for events in eolian
Closed, ResolvedPublic

Description

It appears that there is no convention for how to pass event_info to efl_event_callback_call.
int sometimes passed by value, sometimes passed by reference (compare int events,

structs when declared as value, passed by reference, otherwise by whatever it is defined.

enum Sometimes passed by value with casts, sometimes passed by references. Looks a bit random.

Can you give a little writedown what is suppost to be passed how?

bu5hm4n created this task.Mar 14 2019, 3:04 AM
bu5hm4n triaged this task as TODO priority.
zmike raised the priority of this task from TODO to Showstopper Issues.Mar 18 2019, 8:56 AM

This is currently the most urgent ticket for the 1.22 release.

q66 added a comment.Mar 18 2019, 9:12 AM

Any proposals on this?

q66 added a comment.Mar 18 2019, 9:13 AM

I don't think we currently specify how things are *meant* to be; we need to come up with something.

Hum, there is a side effect to being passed by values vs references. In one case events can not alter the value as it pass there handler. In the other, they can. Maybe we should have a "const" marker to specify explicitly this intent. I think this also make a stronger case to mark efl_event_callback_call a protected function and generate function that will have the proper type checking in their prototype.

Okay this brings up another questions, are we supposed to allow changing of struct fields in event callbacks ?

q66 added a comment.Mar 18 2019, 11:39 AM

I'd say that allowing less is always a good thing, as it allows for fewer errors/mistakes, so if we can do away with it, let's do it

RFC:

When something gets *passed* the type of the input and output is always the same. (where input is a call to efl_event_callback_call, output is the API user that subscribed to a event)
Objects are passed as a normal Eo*, this means event subscribers can call functions on that object.
Iterators & lists should not be allowed as event types.
Structs / buildin-types / containers are *always* passed as const pointer, with exactly ONE level of indirection. The types of hashes and accessors *must* be annotated as const.

In the .eo files everything is always annotated without the reference. (It would be redundant, and we should probebly ban ptr()) from events.

Feel free to give feedback!

q66 added a comment.Mar 18 2019, 12:02 PM

That sounds good to me. So basically the only types allowed are value types, objects and container types, minus list/iterator; everything is annotated without any qualifier, and everything is immutable and passed by reference, except objects. Maybe we should pass objects by const too? So that only @const methods can be called on it. That would make all event data completely immutable.

How are we going to verify and enforce this?

We need to have objects mutable as this is used in our API, see Efl.Input.Hold / Efl.Input.Event. Properties like processed are required to be set.

q66 added a comment.Mar 18 2019, 12:13 PM

Understood. Do we need a way to make objects const when possible?

q66 added a comment.Mar 18 2019, 12:13 PM

Or maybe, make them const by default and some kind of event tag to make it mutable where needed?

Maybe we should just be able to have klass-types const'able... I would not create another new syntax for it tbh.

q66 added a comment.Mar 18 2019, 1:15 PM

I intend to do away with const() as well as ptr() in the current form entirely, so having a new syntax is the better choice for it, besides, immutable by default is a better choice than mutable by default

Don't get me wrong, but this needs to be decided / done for this release, we cannot wait for new syntax, lets just go now with the syntax that is everywhere, and change it afterwards.

q66 added a comment.Mar 18 2019, 1:45 PM

That works too

bu5hm4n closed this task as Resolved.Mar 28 2019, 1:40 AM

Sweetness!