attribute and relvar declaration improvements

[Ostap Cherkashin]

I created another branch with two syntax extensions to the bandicoot language:
1. short(er) syntax for attributes of the same type
2. relational variables declarations using inline types

1. Attributes

Currently if there are several attributes of the same type one would write it like this:
---
rel Points { x: real, y: real }
---

However, there is a more efficient way to write the same declaration without repeating the type name:
---
rel Points { x, y: real }
---

And if there are several attributes, it can be combined like this:
---
rel Points { x, y: real, color: int }
---

Exactly the same approach applies to inline relational type declarations (see below) and to primitive parameters (though I did not merge it with the params branch yet, waiting till it is in the master).

2. Relational Variables

Inline relational type declarations are frequently seen in functions declarations, e.g.:
---
fn GetLogins(): rel { login: string }
{
# ...
}

fn AddUsers(u: rel { name, login: string })
{
# ...
}
---

But these inline types did not work for relational variable declarations. One would need to declare a named relational type first (Point) and then declare a variable, e.g.:

---
rel Point { x, y: real }
chart: Points;
---

With this patch it is now possible to write it like this:
---
chart: rel { x, y: real }
---

Any comments or suggestions?.

- Ostap

[Julius Chrobak]

hi Ostap,

I've had a look at the branch. In general, this looks really good!

I've got only two points:

1) a new mem leak is introduced due to the inline type declaration
for the global variables. An instance of the Head structure is
created but never removed.

2) it seems to me we have an inconsistency for declaration of global
variables in regards to the semicolon:

---
rel Point { x, y: real }

p1: Points;
p2: rel { x, y: real }
---

The semicolon at the end of the line seems artificial and
inconsistent.
I suggest to remove it completely. We could make it mandatory for the
inline
declaration as well but I'm not sure this is a good option.

[Ostap Cherkashin]

Thanks for having a look. Can you provide more details on #1? I can see memleaks but only the ones from my email regarding the primitive params:

---
2.2.) test/relation and test/transaction have memory leaks, but it looks like they were introduced with one of the previous patches
---

WRT #2, making a semicolon obsolete will not fit well with the local variables, e.g.:
---
fn List(): rel { x, y: real }
{
p1: Point

p2: rel { x, y: real }

p1 = storage project(x, y);
p2 = storage project(x, y);
return p;
}
---

If we are to remove it completely it shall also apply to the statements, but (obviously) it is a big change and it needs further discussion. Adding a semicolon seems to make more sense as it is then consistent with the (yet to come) local variables declarations and does not require massive syntax changes.

- Ostap

[Julius Chrobak]

1) the mem leak applies to the new function (add_relvar_inline) in
case of inline declarations:

156 relvar_decl:
157 TK_NAME ':' TK_NAME ';' { add_relvar($3, $1); }
158 | TK_NAME ':' TK_REL rel_head { add_relvar_inline($4,
$1); }

...

552 static void add_relvar_inline(Head *head, const char *var)
553 {
554 if (genv->vars.len == MAX_VARS)
555 yyerror("number of global variables exceeds the maximum
(%d)",
556 MAX_VARS);
557
558 int i = genv->vars.len++;
559 genv->vars.names[i] = str_dup(var);
560 genv->vars.heads[i] = head; /* this is never freed up if the
Head is declared inline */
561 }

When you look at the env_free method, the heads assigned to the global
variables are never freed up:

365 extern void env_free(Env *env)
366 {
367 for (int i = 0; i < env->vars.len; ++i)
368 mem_free(env->vars.names[i]);
369

This was working fine so far because all the heads were kept in env-
>types.heads structure. However, for the new inline declarations the
heads are not in this array and hence never deleted.


2) I'm fine with introducing the semicolon for the inline declaration
at this point. I agree the complete removal triggers more discussion.


On Aug 8, 12:42 am, Ostap Cherkashin <ostap.cherkas...@gmail.com>
wrote:

[Ostap Cherkashin]

I fixed the memleaks and merged it with the master. For the later I had to change the way function parameters are constructed. Also there are some small language cleanups and more tests. Everything is in the attrs_and_decls branch.

[Julius Chrobak]

I've had a look at the branch and it looks good. I do have four
suggestions though. All are related to the error messages modified/
introduced in this branch:

1) some of the messages "xyz already defined" have been changed to
"xyz is already defined" but some were left out. I'd suggest to change
them all to be "xyz is already defined"

2) error message indicating a conflict between a global variable name
and an input parameter is confusing: "global variable '%s' is already
defined". I'd suggest following: "variable name '%s' is already used
for a global variable"

3) in case of the error message "only one relational parameter is
supported at the moment" I'd prefer to remove the "at the moment"
keyword and leave it simpler "only one relational parameter is
supported"

4) for error messages which indicate a maximum config value was
exceeded we use "number of xyz exceeds the maximum (%d)" format. I
suggest to replace "function '%s' defines more primitive parameters
than the configured maximum (%d)" with a shorter version "number of
primitive parameters exceeds the maximum (%d)"



On Aug 18, 12:14 am, Ostap Cherkashin <ostap.cherkas...@gmail.com>

[Ostap Cherkashin]

I changed the error messages and pushed it into the master.