TelluIoT / ThingML

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

C/C++ : an array with a size declared as readonly property #74

Closed lmouline closed 8 years ago

lmouline commented 9 years ago

When we use a readonly property to define the size of an array, there is a bug in the generated code in C/C++ (not in JAVA or JavaScript).

Example :

ThingML Code :

readonly property SIZE : Integer = 10
property array : Integer[SIZE]

C/C++ code :

struct TestReadOnly_Instance {
...
// Variables for the properties of the instance
int TestReadOnly_SIZE__var;
int TestReadOnly_array__var[_instance->TestReadOnly_SIZE__var];
};

Bug : '_instance' was not declared in this scope

brice-morin commented 9 years ago

Yes, we have a test case on that. But @ffleurey did not have time yet to fix that. But you can try to push him :-)

brice-morin commented 8 years ago

related to #4

nharrand commented 8 years ago

I wonder, are read only properties supposed to support the opereation set instance.property? Because if they do, fixing this issue will imply big changes on the way we define things in C.

In my opinion, we need to forbid it at least in c (and then they can be implemented either through const variable, or #define), or forbiding to use properties at all as a size for array properties.

brice-morin commented 8 years ago

Yes, the last chance to set a readonly property is indeed in the configuration using set myInstance.myProperty. At least it would be good that the code generated enforces that readonly properties are indeed read only (so I guess const of #define is the least that we can do).

But compile-time properties can be pretty handy. In some cases (typically when a readonly is affected a simple value e.g. readonly myProperty : Integer = 10, or set myInstance.myProperty = 10) it should be possible to use 10 directly instead of _instance->myProperty__var. Maybe that simplifies things a bit?