gluon-lang / gluon

A static, type inferred and embeddable language written in Rust.
https://gluon-lang.org
MIT License
3.22k stars 146 forks source link

Struct enums are not implemented as enums of records in the codegen. #729

Open franekp opened 5 years ago

franekp commented 5 years ago

Let's consider the following enum in Rust:

enum Point {Cartesian {x: f64, y: f64}, Polar {r: f64, phi: f64} }

Currently the codegen assumes that the corresponding Gluon definition looks like this:

type Point = | Cartesian Float Float | Polar Float Float

However, a reasonable person would expect the following Gluon type instead:

type Point = | Cartesian {x: Float, y: Float} | Polar {r: Float, phi: Float}

Is there any reason to not use this more intuitive form?

Context: https://github.com/gluon-lang/gluon/blob/master/codegen/tests/derive_getable.rs#L107 https://github.com/gluon-lang/gluon/blob/master/codegen/tests/derive_pushable.rs#L235

Laegluin commented 5 years ago

Is there any reason to not use this more intuitive form?

I tried really hard to come up with the reason I did it this way, but I'm really not sure anymore. I think it was mainly because these two variants would the map to the same Gluon record:

enum Enum {
    StructVariant { x: f64, y: f64 },
    TupleWithStruct(Point),
}

struct Point {
    x: f64,
    y: f64,
}

... but that's still not really a good justification. I guess people just rarely use struct variants, so nobody noticed.

I would agree that your solution is much more reasonable! :)

Marwes commented 5 years ago

The record form does require an extra allocation to create the record, but I agree that is a better form 👍