gluon icon indicating copy to clipboard operation
gluon copied to clipboard

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

Open franekp opened this issue 5 years ago • 2 comments

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

franekp avatar May 12 '19 18:05 franekp

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! :)

Laegluin avatar May 12 '19 20:05 Laegluin

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

Marwes avatar May 12 '19 20:05 Marwes