marcobambini / gravity

Gravity Programming Language
https://gravity-lang.org
MIT License
4.3k stars 229 forks source link

Enum numbering bug #168

Closed MatanSilver closed 6 years ago

MatanSilver commented 7 years ago

When running:

enum testenum {
    num0,       // default to 0
    num1,           // default to 1
    num2,       // default to 2
    num3 = 3,
    num4=2,
    num5            // 4?
};

func main() {
    System.print("num0: " + testenum.num0);
    System.print("num1: " + testenum.num1);
    System.print("num2: " + testenum.num2);
    System.print("num3: " + testenum.num3);
    System.print("num4: " + testenum.num4);
    System.print("num5: " + testenum.num5);
}

The output is:

num0: 0
num1: 1
num2: 2
num3: 3
num4: 2
num5: 3
RESULT: NULL (in 0.0880 ms)

When I suspect it should be something like:

num0: 0
num1: 1
num2: 4
num3: 3
num4: 2
num5: 5
RESULT: NULL (in 0.0880 ms)

In other words, the earlier values should check ahead to see if their sequential numbers automatically assigned are soon to be taken away.

marcobambini commented 7 years ago

Thanks @MatanSilver, I'll fix it pretty soon.

hallzy commented 7 years ago

The output of your code looks okay to me. It is the same output you would get for the equivalent C code. Why were you expecting 4 for num2, and 5 for num5?

MatanSilver commented 7 years ago

The way I see it, num2 should be 4 because, though 2 is the next in the sequence of numbers, later in the enum the number 2 is manually used up so it should be avoided, even before hand, so there are no duplicates. And then num5 should be 5 because once num2 is set to 4, 4 is used up so 5 becomes the next lowest free number. I actually didn't know that C worked this way, but that's interesting to hear about--I'll have to try it in C too, for myself. Sorry if my example was unclear in any way

MatanSilver commented 7 years ago

@hallzy Wow you're right, I tried it just now. Well, I learn something new every day. Thanks!

marcobambini commented 7 years ago

I am not sure I like the fact that two enums can share the same value...

MatanSilver commented 7 years ago

That was my first thought too. It seems you could have some undefined behaviors if you had two equal values and then checked for one or the other in a conditional--which value is it? I mean, if this is an issue that needs to be solved, I feel like there are a couple approaches:

  1. Add some sort of "peek-ahead," so that for every compiler-set enum value, it makes sure that the value it picks is not set anywhere later
  2. Maintain compatibility with C, which would mean enforcing that all compiler-set enum values come before user-set ones. This would avoid the necessity for the "peek-ahead," if compiler simplicity is the goal, but at the expense of an artificial constraint on the order of values
hallzy commented 7 years ago

Ah, I get your example now. Is there any other language that works that way though? I think it could be confusing to change the behaviour to be different from all other languages (unless there are languages I don't know about that act this way).

If it is the case that all or almost all languages treat enums as C does, then I think we should leave it. We could add a special gravity enum that works how you expected it.

There are probably use cases for both. Like with your example you are basically ensuring that all values are unique which could be useful and prevent human error.

The way enums are handled currently is useful too. For instance if you wanted to start the numbers at 1 instead of 0, all you would ned to do is make the first element equal 1 and then the rest is incremented.

For readability you can also add min and max values to enums in c.

typedef enum {
  Min,         // 0
  Red=0,       // 0
  Blue,        // 1
  Green,       // 2
  Max=Green    // 2
} colours;
MatanSilver commented 7 years ago

It looks like C, C++, Java, and Crystal use c-like enum numbering. From what I can tell, Rust actually doesn't let you mix explicit or implicit "discriminators," you can only use one or the other in a native enum. Rust does let you mix explicit and implicit discriminators if you use a "C-style" enum, but then it complains if you do something like this:

enum TestEnum {
    Num1,
    Num2,
    Num3 = 1
}

and throws the following error:

   |
20 |     Num2,
   |     ---- first use of `1isize`
21 |     Num3 = 1
   |            ^ enum already has `1isize`

I think the approach @hallzy suggested, of having a "safe" and an "unsafe" enum is a good compromise. Or an alternative is to have a compiler warning but no error?

marcobambini commented 7 years ago

Gravity does not need to treat enums as C does. For example C enums only support integer values while Gravity enums already support any type of literals like string, int, bool, float and so on.

Automatic enumeration of type int is supported for enum entries without a default value and I really think we should treat duplicated values (implicit or explicit) as a static compilation error.

An enum like:

enum testenum {
    num0,       // default to 0
    num1,       // default to 1
    num2,       // default to 2
    num3 = 3,
    num4 = 2,
    num5
};

should return an error because num4 contains a duplicated value.

While an enum like:

enum testenum {
    num0,       // default to 0
    num1,       // default to 1
    num2,       // default to 2
    num3 = 3,
    num4 = 80,
    num5
};

should be resolved with num5 = 81 (a simple max(enum_values)+1 could be used).

hallzy commented 7 years ago

ya that makes sense. My main worry was just that if someone is making an assumption about how enums work based on how they work elsewhere they may be confused when an error occurs because they have duplicated values. But I see your point, and honestly, I can't think of any real good reasons to have duplicated values in an enum anyways, unless you wanted to make clear what your return codes mean but you still want to return the same number or something but that seems like a stretch.

I just want to clear something up though... so if we change how enums work in gravity what would happen here:

enum testenum {
    num0,        // 0
    num1,        // 1
    num2,        // 2
    num3 = 3,
    num4 = -1,
    num5;        // ??
};

C-style would say num5 is 0, and according to your second example it would be 0 as well and if we aren't allowed duplicates then this will be a problem. Or are we just going to have to know better than to order our enums this way.

Either that or the new enum will need to be smarter and before it assigns a value it will check if it already exists, and if it has it checks the next number and so on until it finds one that isn't assigned... which would make num5 = 4

marcobambini commented 7 years ago

According to my second example, num5 should be 4 (max(enum_values)+1)

hallzy commented 7 years ago

Oh right... duh. Missed that part.

In that case I see no reason why that would not work apart from simply being confusing at first. That being said, most people would probably never find a situation where that is a problem anyways

marcobambini commented 6 years ago

It turns out that it seems to be a good idea to allow duplicated enum values (https://stackoverflow.com/questions/11412516/why-can-two-different-enum-enumeration-constants-have-the-same-integer-value) so I am going to allow this behaviour.

marcobambini commented 6 years ago

Closed by https://github.com/marcobambini/gravity/commit/431ace192d13d60c08746d7cbaa54303723e3cea