Page MenuHomePhabricator

mono-bindings: Missing setter for struct fields
Open, Showstopper IssuesPublic


I cannot write to Eina.Size2D.W:

Property or indexer `Eina.Size2D.W' cannot be assigned to (it is read-only)`

Indeed, this is the definition in eina_types.eot.cs:

/// <summary>X position in pixels, from the top-left corner.<br/>Since EFL 1.22.</summary>
public int W { get => w; }
/// <summary>Y position in pixels, from the top-left corner.<br/>Since EFL 1.22.</summary>
public int H { get => h; }

The source eina_types.eot is:

struct @extern Eina.Size2D {
   [[A 2D size in pixels.

     @since 1.22
   w: int; [[X position in pixels, from the top-left corner.]]
   h: int; [[Y position in pixels, from the top-left corner.]]

Therefore the setter is missing.

segfaultxavi triaged this task as Showstopper Issues priority.
jptiz added a comment.Feb 7 2020, 9:24 AM

As far as I can see from struct_definition.hh, fields are intentionally read-only (don't know the reason, tho), while properties may be writable. Should we change that rule and add a set for them, then? Where could I find the reasoning about who should be read-only, etc.?

jptiz added a comment.EditedFeb 7 2020, 9:28 AM

Oops, just realized actually structs don't have properties ( Maybe changing an eo-struct would require creating a new one with modified values, letting mutability only to classes?

I don't know why that was done that way, but seems a bit arbitrary to me. Maybe @felipealmeida knows?

I think this is just someone missing the setter. The setter should exist IMO. @jptiz can you look at this monday? Thanks.

jptiz added a comment.EditedFeb 10 2020, 6:06 AM

@felipealmeida So, I've found rEFL581bec9598943cc9274dfe7db1a73a4c878c3cdd, which makes struct immutable because of C# recommendations (and there's the reasoning I was looking for, too). Should we propose to make EO structs generate classes instead, then? Where could I read/who to talk about if it would or not go against the idea of EO structs?

@jptiz No. Leave it as is then. @segfaultxavi do you agree with the patch reasoning?

If C# devs are not used to mutable structs, then we should not provide them, yeah.
What do you suggest we do, though? EO structs ARE mutable.

IMO Eo structs are not mutable per se. C and C++ structs are mutable.

So how's the user of a EO struct supposed to put the information inside it?

Just to add some (possibly) useful info:

  1. There was some previous discussion about this in #efl# IRC Channel regarding the default behaviour: since EO is (sort of) a binding generator, maybe it doesn't have to define things such as mutability per se. It would just translate to the desired language's features - then the discussion would become "what are the appropriate features?".
  1. If we agree with 1., what would make more sense?
    • Option 1: Translate Eo structs to C# classes, copying mutable behaviour from C structs (altough enforcing heap allocation instead of a possible alloc. in stack);
    • Option 2 (current): Translate straight-forwardly into C# structs (as it is now), but that would mean they would be immutable in the C# side, so the programmer would be on his own to deal with it (e.g. creating a new instance based on the original - see below).

For Option 2, then a workaround for @segfaultxavi's case (Size2D.W set) would be like the following:

void Foo(Bar something) { // assuming `Bar` is a class and `something.Size` exists
    // eventually having to double something's size:
    something.Size = new Size2D {
        W = something.Size.W * 2,
        H = something.Size.H,

Which would still be idiomatic, just would need some more work/considerations from the programmer's side.

  1. As for current usage, as far I as searched, every usage of Eo-structs seems to be in contexts that would be fair to be immutable (*Args for event callbacks, for example).

I understand the C# reasoning. But I still think being able to write size = new Size2D(w,h); but not size.W = w; is extremely weird.
However, I am not a C# person. Maybe we need also input from the C# users? @woohyun @YOhoho ?