Re: [PATCH] fixup! docs: spec: add variables and constants specification

[Johnny Richard <johnny@johnnyrichard.com>]

Thanks a lot for your contribution, I very impressed you managed to send
a really nice PATCH.

Sorry for that, but there is another Patchset version which was applied
to the main branch.  I think Carlos forgot to mention this Patchset was
SUPERSEDED[v2] by version 2.

v2: http://lists.johnnyrichard.com/olang/5fzsolce5h42aa6udppiwezgbzeqerkde3xvyilidnkcjaho2j@ygxd7ugzck4m/T/#m0f5cc6f2d49835ec83d4fd5c24d97d2597cb5363

However, I will leave my thoughts here but (we should try to address the
current implementation problems in a new patches).

On Thu, Apr 11, 2024 at 07:39:58PM -0300, ricardo_kagawa@disroot.org wrote:
> From: Ricardo Kagawa <ricardo_kagawa@disroot.org>
> > Since this patch adds support to assignments, lets also add support to
> > all assignment operators like "-=" "+=" "<<=" and so on.
> 
> I disagree. Support for those assignment operators should only be added
> when both assignments and operators are defined, and none of these
> operators are defined yet.

Sure... Unfortunately we don't have the specification for operators but
they are already implemented into the language parser (I was planning to
submit the assignment operators spec after this patch get applied).

As soon as I send the Patch we can discuss on top of it anything related
to assignment operators.

> > This patch lacks support to the following valid assignment expression
> > (which I think adds flexibility to the language):
> > 
> >     var x: u32 = a = b = c = 1
> 
> Personally, I don't like this idiom, but I wouldn't stop you from adding
> it. Johnny's patch actually already enables this, and also the
> following (for clarity, of course the `if` statement does not exist yet,
> but it is included here as an example of what might come in the future):
> 
> ```
> var x
> if (x = next()) {
>   return x
> }
> ```
> 
> Which is something I don't like either, just as much.

Carlos and I discussed it and we also agreed on removing this assignment.
The patch v2 has removed it.

> So, in my opinion, the decision should hinge on the compiler's ability
> to properly validate the constant's immutability, and whether or not
> late initialization should be allowed (complicating semantic checks).

I prefer to not do a late initialization as well (it will make things
simpler).  I prefer to make the assignment mandatory for constants.

> ---
> Discussions:
> 
> - Are you planning to hoist declarations, or are declarations required
>   to be placed before their use?

We are planning to have declarations being placed before their use.

> - Are you planning to include type inference for variable declarations?

No.  Everything should be verbose and explicit.

> -- >8 --
> 
> - Moved statements common to the translation unit and function bodies to
>   their own non-terminal.

Good.

> - Restored Carlos' definition of variable and constant definitions, as
>   they are not quite the same, structurally and semantically.
>   Personally, I'd rather constants to be initialized immediately, but
>   you may have a different opinion on this.

We also prefer to have the constants being initialized immediately.

> - Moved some non-terminals from the "Statements" section to the
>   "Functions" section, as there will be some statements that are
>   function-body specific, some that are translation-unit specific, and
>   some that are common to both.

I liked it.

> - Renamed "assign" to "assignment", for better wording.

Nice.

> - Renamed <variable-assign> from Carlos' patch to
>   <variable-initializer> (must not be used in <assignment-expression>).

Nice.

> 
> Signed-off-by: Ricardo Kagawa <ricardo_kagawa@disroot.org>
> ---
>  docs/pages/language-specification.md | 36 ++++++++++------------------
>  1 file changed, 13 insertions(+), 23 deletions(-)
> 
> diff --git a/docs/pages/language-specification.md b/docs/pages/language-specification.md
> index 2650dd9..b218462 100644
> --- a/docs/pages/language-specification.md
> +++ b/docs/pages/language-specification.md
> @@ -24,15 +24,14 @@ language.
>  (* Entry Point *)
>  <translation-unit>     ::= (<ows> <external-declaration> <ows> (<end-of-statement> | <end-of-file>))*
>  
> -<external-declaration> ::= <function-definition> 
> -                         | <variable-definition>
> -                         | <assign-expression>
> +(* Translation Unit *)
> +<external-declaration>  ::= <common-statement> | <function-definition>
>  
>  (* Variables *)
> -<variable-definition>  ::= <variable-qualifier> <ws> <variable-name> <ows> ':' <ows> <type> (<ows> <assign-operator> <ows> <expression>)?
> -<variable-qualifier>   ::= 'var'
> -                         | 'const'
> -<variable-name>        ::= <identifier>
> +<variable-definition>   ::= 'var' <ws> <variable-name> <ows> ':' <ows> <type> <ows> <variable-initializer>?
> +<const-definition>      ::= 'const' <ws> <variable-name> <ows> ':' <ows> <type> <ows> <variable-initializer>

After discussing with Carlos I regretted my suggestion about combining
_var_ and _const_.  Today we have both combined.  But separate them
makes a lot easier to implement.

Could you please prepare a new patch over the _main_ branch with this
change (if Carlos agree of course)?

> +<variable-name>         ::= <identifier>
> +<variable-initializer>  ::= '=' <ows> <expression>
>  (* Functions *)
>  <function-definition> ::= 'fn' <ws> <function-name> <ows> <function-parameters> <ows> ':' <ows> <return-type> <ows> <function-body>
> @@ -40,27 +39,18 @@ language.
>  <function-parameters> ::= '(' <ows> ')'
>  <return-type>         ::= <type>
>  <function-body>       ::= <block>
> +<block>               ::= '{' <ows> <statement> <ows> (<end-of-statement> <ows> <statement> <ows>)* <end-of-statement>? <ows> '}'
> +<statement>           ::= <common-statement> | <return-statement>
> +<return-statement>    ::= 'return' <ws> <expression>
>  
>  (* Statements *)
> -<block>               ::= '{' <ows> <statement> <ows> (<end-of-statement> <ows> <statement> <ows>)* <end-of-statement>? <ows> '}'
>  <end-of-statement>    ::= ';' | <line-break>
> -<statement>           ::= <return-statement> | <variable-definition> | <assign-expression>
> -<return-statement>    ::= 'return' <ws> <expression>
> +<common-statement>    ::= <variable-definition> | <const-definition> | <assignment-expression>
>  
>  (* Expressions *)
> -<expression>          ::= <integer> | <identifier> | <assign-expression>
> -<assign-expression>   ::= <variable-name> <ows> <assign-operator> <ows> <expression>
> -<assign-operator>     ::= '='
> -                        | '*='
> -                        | '/='
> -                        | '%='
> -                        | '+='
> -                        | '-='
> -                        | '<<='
> -                        | '>>='
> -                        | '&='
> -                        | '^='
> -                        | '|='

As I said before, these _assignment operators_ are already implemented.
I have implemented it before we start the language spec.  I will prepare
a patch to fix the spec.

> +<expression>            ::= <integer> | <variable-name> | <assignment-expression>
> +<assignment-expression> ::= <variable-name> <ows> <assignment-operator> <ows> <expression>
> +<assignment-operator>   ::= '='
>  
>  (* Identifiers *)
>  <type>                ::= 'u32'
> -- 
> 2.44.0
>