Re: [PATCH olang 2/5] checker: scope: populate symbol's type

[Johnny Richard <johnny@johnnyrichard.com>]

On Sat, Sep 21, 2024 at 08:24:56AM GMT, Carlos Maniero wrote:
> Persisting the symbol type will be very convenient for codegen once this
> information can be used to determine how many bytes that symbol
> requires.

I also agree.  I would change a little bit here and make the checker
calculate the size in bytes and add it to the symbol.

> diff --git a/src/checker.c b/src/checker.c
> index 3b713f7..f5068e0 100644
> --- a/src/checker.c
> +++ b/src/checker.c
> @@ -23,6 +23,9 @@
>  static void
>  populate_scope(checker_t *checker, scope_t *scope, ast_node_t *ast);
>  
> +static void
> +populate_types(checker_t *checker, ast_node_t *ast);
> +
>  checker_t *
>  checker_new(arena_t *arena)
>  {
> @@ -44,6 +47,7 @@ checker_check(checker_t *checker, ast_node_t *ast)
>      assert(ast);
>  
>      scope_t *scope = scope_new(checker->arena);
> +    populate_types(checker, ast);
>      populate_scope(checker, scope, ast);

We are traversing the tree twice to populate_scope.  I think the
populate scope can set the type on scope at this moment.

>  
>      // TODO: traverse the ast tree to verify semantics
> @@ -107,7 +111,8 @@ populate_scope(checker_t *checker, scope_t *scope, ast_node_t *ast)
>  
>          case AST_NODE_VAR_DEF: {
>              string_view_t id = ast->as_var_def.identifier;
> -            symbol_t *symbol = symbol_new(checker->arena, id);
> +            type_t type = ast->as_var_def.type;
> +            symbol_t *symbol = symbol_new(checker->arena, id, type);

Let's include the size in bytes for the symbol. I believe this will
enhance our symbol and make it easier to verify in the next steps

>  
>              scope_insert(scope, symbol);
>              ast->as_var_def.scope = scope;
> @@ -124,3 +129,79 @@ populate_scope(checker_t *checker, scope_t *scope, ast_node_t *ast)
>              return;
>      }
>  }
> +
> +static void
> +evaluate_type(type_t *type)
> +{
> +    if (string_view_eq_to_cstr(type->id, "u32")) {
> +        type->kind = TYPE_PRIMITIVE;
> +        type->as_primitive = TYPE_U32;
> +        return;
> +    }
> +
> +    type->kind = TYPE_USER_DEFINED;
> +}
> +
> +static void
> +populate_types(checker_t *checker, ast_node_t *ast)
> +{

If we set extra data to symbol's type we don't need to traverse the ast
again here.

Could you please enlighten the motivation behind setting extra data to
types on AST nodes?

> diff --git a/src/type.h b/src/type.h
> new file mode 100644
> index 0000000..855cd83
> --- /dev/null
> +++ b/src/type.h
> @@ -0,0 +1,39 @@
> +/*
> + * Copyright (C) 2024 olang maintainers
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <https://www.gnu.org/licenses/>.
> + */
> +#ifndef TYPE_H
> +#define TYPE_H
> +typedef enum
> +{
> +    TYPE_PRIMITIVE,
> +    TYPE_USER_DEFINED
> +} type_kind_t;

I don't see other kinds in the future.  What do you think about adding a
boolean property named "is_primitive"?