[Bug] [ABNF] Ordering and commas for member declarations

[acoglio]

🐛 Bug Report

The current rules for circuit member declarations require commas to separate each member, both variables and functions:

member-declaration = member-variable-declaration
                   / member-function-declaration

member-variable-declaration = identifier ":" type

member-function-declaration = function-declaration

circuit-declaration = *annotation %s"circuit" identifier
                      "{" member-declaration *( "," member-declaration ) "}"

But this page https://developer.aleo.org/aleo/concepts/programs shows that member functions are not followed by commas. This seems reasonable, so we should update the grammar rules, but here are a few questions:

• Should all the member variables precede all the member functions in a circuit declaration, or can they freely intermixed?
• Since commas look more like "separators" than "terminators", should semicolons be used instead, as the (ending) part of every member variable declaration?
• If we stay with commas, and the last member of a circuit is a member variable, can the final comma be omitted and/or included?

[gluax]

I would say member variables should precede member functions and the last one should always terminate with a comma. Though I'm not opposed to the ending symbol being a semi-colon since we define our member variables and functions in the same scope.

I like the idea of these restrictions being placed into the grammar and language itself because it enforces a bit of styling at a lower level.

@acoglio @collinc97 thoughts?

[acoglio]

I have no strong opinions, but my vote would be to require member variables to be terminated by semicolon instead of comma and to allow intermixing of member variables and functions. Style guides could suggest to put all member variables first and then all member functions in general; but perhaps there are cases in which it may make more sense to group things differently (although that may call for splitting a large circuit into smaller ones).

[gluax]

@damirka @howardwu @Protryon would like to see where you guys weigh in on this as well!

[collinc97]

I'm a fan of member variables with an optional trailing comma. No commas after member functions.

circuit Foo {
    x: u32,
    y: u32
}

circuit Foo {
    x: u32,
    y: u32,
}

I think both should be allowed.

[gluax]

I think the strangeness in that is that we allow variable and function definitions in the same block @collinc97. Our style is more c++ similar where define both in a block. But c++ requires the semi-colon here.

[acoglio]

Also similar to Java -- mixed field and methods, with fields terminated by semicolon.

A Rust struct has only fields, no methods.

[gluax]

In the best interest of time and not rushing this discussion, for DP2 should I just make it so that the abnf grammar just says no commas after member functions @acoglio @collinc97? Could leave this PR open and discuss more then.

[acoglio]

We could do that, but as I think of grammar rules for that, they seem a bit awkward (unless I'm missing something):

member-variable-declaration = identifier ":" type

member-variable-declaration-with-comma = member-variable-declaration "," ; seems awkward

member-function-declaration = function-declaration

member-declaration = member-variable-declaration-with-comma
                   / member-function-declaration
; seems awkward that a member-variable-declaration-with-comma is a member-declaration,
; but a plain member-variable-declaration is not a member-declaration;
; we could alternatively use member-declaration-that-can-be-followed instead of member-declaration,
; but that's very long, or member-declaration-followable but I couldn't find 'followable' in the dictionary

circuit-declaration = *annotation %s"circuit" identifier
                      "{" *member-declaration [ member-variable-declaration ] "}"

@gluax Maybe you have thought of a more elegant formulation of the rules?

Note that this allows a circuit with no members, which the current rules disallow, but I believe the parser allows. Ideally that should be a separate issue, but we may take care of this while we are at it.

[acoglio]

Maybe we could turn things around a bit:

member-variable-declaration-without-comma = identifier ":" type

member-variable declaration = member-variable-declaration-without-comma ","

member-function-declaration = function-declaration

member-declaration = member-variable-declaration
                   / member-function-declaration

circuit-declaration = *annotation %s"circuit" identifier
                      "{" *member-declaration [ member-variable-declaration-without-comma ] "}"

I think this is less awkward, but still a bit.

[acoglio]

This is what I would vote for:

member-variable-declaration = identifier ":" type ";" ; i.e. require semicolon at the end

member-function-declaration = function-declaration

member-declaration = member-variable-declaration
                   / member-function-declaration

circuit-declaration = *annotation %s"circuit" identifier "{" *member-declaration "}"

[gluax]

The last one makes a lot of sense @acoglio. Although if we want variables to come before functions I think we would need to do something like this in the future.

member-declaration = 1*member-variable-declaration *member-function-declaration

[collinc97]

I'm a fan of member variables with an optional trailing comma. No commas after member functions.

circuit Foo {
    x: u32,
    y: u32
}

circuit Foo {
    x: u32,
    y: u32,
}

I think both should be allowed.

Reopening this discussion, is it impossible in the ABNF grammar to have rules that allow this case?

I'm asking because it seems like the purpose of adding semicolons to the end of circuit members is just to have complete ABNF rules.

[acoglio]

It's possible to have ABNF rules that allow that, such as the ones at #842 (comment).
They are a bit more complex/awkward than the ones at #842 (comment).
The point that @gluax and I were making is that the commas, and trailing commas, tend to be used in sequences of homogeneous entities separated by commas (such as fields in a Rust struct), but a Leo circuit contains heterogeneous things (variables and functions), with one of the two two things (functions) not being followed by comma. So the latter is more similar to a Java or C++ class declaration, which has fields and methods.

[acoglio]

By the way, I just noticed (by accident) that it is also allowed to have no commas (or semicolons). The following works with the latest Leo:

circuit Point {
    x: i8
    y: i8
    function new(x: i8, y:i8) -> Self {
        return Self {x, y};
    }
    function move(mut self, dx: i8, dy: i8) {
        self.x += dx;
        self.y += dy;
    }
}

function main(a: i8, b: i8) -> i8 {
    let p = Point::new(a, b);
    p.move(3, 2);
    return p.x + p.y;
}

[gluax]

The no commas make sense since we don't currently check if something is the last member variable before member functions, so they are effectively just optional at the moment.

So if I remember correctly, the discussion after the end of the last meeting was to use semi-colons and commas. Then after we have a proper warning system, we can go ahead and give people a warning about the comma deprecation.

We can fix the commas and add the semi-colons here, and then update the documentation.

[acoglio]

@gluax Your summary sounds good to me.