Allow adding methods to C structs

[oprypin]

Making wrappers for small structs that are used a lot is unreasonable. It would be nice to make C lib types fit better into the language. The most important step is allowing to extend them with methods.

[Arcnor]

I was just going to add this issue myself :smile:

When creating C structs inside a lib block, they don't get any special treatment AFAIK. So that means I still have to pass them through pointerof when using them as pointers for example. It will be nice if we can add to_unsafe or a constructor, etc

[pyr0hu]

+1

[asterite]

So, some points about this:

1) It's currently possible to add methods to C structs/union, but you must do this by reopening a type from outside the lib:

lib LibFoo
  struct MyStruct
    x : Int32
    y : Int32
  end

  union MyUnion
    x : Int32
    y : Int32
  end
end

struct LibFoo::MyStruct
  def foo
    x + y
  end
end

struct LibFoo::MyUnion
  def foo
    x + y
  end
end

my_struct = LibFoo::MyStruct.new x: 1, y: 2
pp my_struct.foo #=> 3

my_union = LibFoo::MyUnion.new x: 1, y: 2
pp my_union.foo #=> 4

However, this almost works by accident: at one point we made C structs/unions inherit Struct so they have a nice to_s, and that made it possible to do the above (only that I realized this today).

Because this was not intended, if you use/assign an instance variable that's not either "@x" or "@y" the compiler will crash, but we can later make it give a nicer error.

2) My main concern, though, is that C structs/unions are an implementation detail of a library and users of that library shouldn't have to know this. In particular, C structs/unions have different semantics than regular struct/classes. For example, their constructor (new) zeroes everything, you can't add instance variables to them, etc. Also, they are not documented by crystal doc.

But I understand that, for example, SDL might return an Event struct/union and that these will always be created by the library, so it doesn't matter that constructors work different for this type. So adding methods to it wouldn't be a big deal.

Maybe if there's an alias outside the lib that points to a struct/union type inside the lib we can make it be documented, I don't know.

On the other hand, when you wrap these libraries in Ruby/Python I think you create these extra wrappers because you have no other way, you obviously can't expose C structs/unions to python code directly. Well, you can think that the same is true in Crystal: just because you can write C bindings in Crystal it shouldn't mean C types should be exposed to end-users. They are just there so the glue can be made easier.

[oprypin]

You say lib structs are an implementation detail. But you could say so about normal structs too, just that they are slightly different. If they were more similar (which is the point of our conversation), this wouldn't be a problem.

The only good argument is unability to add instance variables. But I don't see it as a problem. Such things are to be expected from lib bindings. And the rest can be fixed.

And what is the alternative? Writing tons of boilerplate? Getter and setter for each attribute? Constructors? Wrapping and copying everywhere? (and don't underestimate the amount of copying) You'd also have to use to_unsafe directly because it is not invoked when passing to a non-pointer argument.

And you can't talk about Ruby/Python, because... don't forget the main point of Crystal. If performance wasn't a problem, I would still be happily using Python for everything.

---

Anyway, what you showed works perfectly. The biggest gotcha I found is, indeed, that the constructor zeroes everything, and it is impossible to override it. And if other constructors are made and you try to specify a named argument to new, it tries to apply the struct's "natural" constructor anyway.

initialize with zero arguments never gets called, and the following code crashes:

lib L
  struct T
    x: Int32
  end
end

struct L::T
  def self.new()
    self.new(x: 7)
  end
end

p L::T.new()

Using a different class method name works, but I'd like to see a better way. Something should be done so the "natural" constructor doesn't get such a high priority.

---

Another thought I have... why not take the unification even further and allow making these structs outside of lib. These would be "optimized" structs (although I'm not sure if they would provide any noticable benefit).

[asterite]

You can do this in the way I've shown, by reopening the struct/union outside the lib. However, I wouldn't encourage this and I'd use it only when strictly necessary. So I'll close this.

[oprypin]

So you're leaving this in a dire state.

If you define a new for the struct, the program crashes. If you don't, you allow the user to initialize the struct to all zero bytes.

[oprypin]

And I'm supposed to just pick one of these 2 horrible options in my library, while also defining ugly top-level functions that serve as constructors.

[asterite]

You should expose proper crystal wrappers. lib is unsafe by definition. Maybe we can change the current behaviour to disallow adding methods to lib declarations.

[oprypin]

I've already stated that making pointless wrappers is... pointless.

If you want to make library development more painful, if you want to make high performance impossible to achieve, then sure, remove it.

Or maybe this bug that crashes the program (not because it's unsafe but because of incorrect compilation) could just be fixed.

[asterite]

If it crashes it's a bug and we'll eventually fix it :-)

[oprypin]

5cb0a604eb4211445ccc8ed66cbbeb90a757ad71 allows redefining the constructor, thanks!

lib L
  struct T
    x : Int32
  end
end

struct L::T
  def initialize(@x=7)
  end
end

p L::T.new    #=> L::T(@x=7)
p L::T.new(4) #=> L::T(@x=4)