TelluIoT / ThingML

The ThingML modelling language
https://github.com/TelluIoT/ThingML
Apache License 2.0
101 stars 32 forks source link

enumeration without @enum_val specified gives 0 as value to all elements #220

Closed antons1 closed 5 years ago

antons1 commented 6 years ago

When making an enum, for instance:

enumeration testEnum
@c_type "uint8_t"
@c_byte_size "1"
{
    ENUMVAL1
    ENUMVAL2
    ENUMVAL3
}

all three elements will get value 0, at least when compiling to arduino and posix:

// Definition of Enumeration  testEnum
#define TESTENUM_ENUMVAL1 0
#define TESTENUM_ENUMVAL2 0
#define TESTENUM_ENUMVAL3 0

Adding an @enum_val to each of them fixes it, but the annotation does not seem to be required, and I would assume that the default behaviour of an enum is to automatically assign unique values to each of the elements if none are specified.

I also get mismatched input '<' expecting '{' if I try to put the byte size in the enum name: enumeration testEnum<1>

The file I tested with: (with a .txt extension to make github allow upload) enum.thingml.txt

jakhog commented 6 years ago

Can you try it for Java/NodeJS as well? It might be a C-compiler issue only...

jakhog commented 6 years ago

Seems like this is C specific. And it is not actually a bug, see: https://github.com/TelluIoT/ThingML/blob/685f38a19c05a32e7755e7499cee94a9d22a2b33/compilers/c/src/main/java/org/thingml/compilers/c/CCompilerContext.java#L409-L416

But the fact that the error is printed with stderr is a bug.

Anyway, I think we should try to auto-generate enum values (C should do that automatically for us?)

brice-morin commented 6 years ago

We might want to update the language so that enumerations have a type, reffering to a ThingML datatype, which will give the size of the literals, that we need e.g. when we serialize. And we could also force the fact that enumeration literals should be initialized with a.... literal. Something like

enumeration testEnum is Integer
{
    ENUMVAL1 = 0
    ENUMVAL2 = 1
    ENUMVAL3 = 2
}

@ffleurey, all: what do you think?

IMHO, this should avoid common problems when dealing with enumerations, by forcing people to be more precise, and avoiding relying on obscure annotations and on some default behavior hard-coded in the compilers.

jakhog commented 6 years ago

Originally, I was thinking that we should either:

  1. Keep enumerations as some abstract set of values, that are not specifically mapped to anything you know. Kind of like they are today.
  2. Do as @brice-morin suggests and make it mandatory to map it to a type, and values to actual literals

With 1, it would be easy to define, but it would not be portable (i.e. not possible to send over protocols). With 2, it would be portable, but a bit more painful to define.

But, when I think about it more, I think we can (with some work on the Checker) have the best of both.

I suggest that we keep the possibility to define enums as they are today, but add @brice-morin suggestion as an optional syntax. Then we can use the checker to:

  1. Verify that enums defined with is Integer (I might prefer as Integer myself), have literals assigned to the values as in the comment above
  2. Verify that enums sent in messages, are defined with a Type mapping
  3. Verify that if you try to assign to a variable of an enum type, the value is of correct enum literal, or the value that it is mapped to.
brice-morin commented 6 years ago

Sounds good to me!