[Mesa-dev] [PATCH 07/32] glsl: Refactor out processing of structure fields

[Paul Berry]

On 22 January 2013 00:51, Ian Romanick <idr at freedesktop.org> wrote:

> From: Ian Romanick <ian.d.romanick at intel.com>
>
> This will soon also be used for processing interface block fields.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/glsl/ast_to_hir.cpp | 42 +++++++++++++++++++++++++++++-------------
>  1 file changed, 29 insertions(+), 13 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index c432369..bce3488 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -4014,35 +4014,36 @@ ast_type_specifier::hir(exec_list *instructions,
>  }
>
>
> -ir_rvalue *
> -ast_struct_specifier::hir(exec_list *instructions,
> -                         struct _mesa_glsl_parse_state *state)
> +unsigned
> +ast_process_structure_or_interface_block(exec_list *instructions,
> +                                        struct _mesa_glsl_parse_state
> *state,
> +                                        exec_list *declarations,
> +                                        YYLTYPE &loc,
> +                                        glsl_struct_field **fields_ret)
>

The contract with the caller isn't obvious to me from this function
declaration.  Can we have a short comment above the function saying that
the return value is the number of fields and that *fields_ret receives a
pointer to a newly allocated array with that size?

With that change, this patch is:

Reviewed-by: Paul Berry <stereotype441 at gmail.com>


>  {
>     unsigned decl_count = 0;
>
> -   /* Make an initial pass over the list of structure fields to determine
> how
> +   /* Make an initial pass over the list of fields to determine how
>      * many there are.  Each element in this list is an
> ast_declarator_list.
>      * This means that we actually need to count the number of elements in
> the
>      * 'declarations' list in each of the elements.
>      */
> -   foreach_list_typed (ast_declarator_list, decl_list, link,
> -                      &this->declarations) {
> +   foreach_list_typed (ast_declarator_list, decl_list, link,
> declarations) {
>        foreach_list_const (decl_ptr, & decl_list->declarations) {
>          decl_count++;
>        }
>     }
>
> -   /* Allocate storage for the structure fields and process the field
> +   /* Allocate storage for the fields and process the field
>      * declarations.  As the declarations are processed, try to also
> convert
>      * the types to HIR.  This ensures that structure definitions embedded
> in
> -    * other structure definitions are processed.
> +    * other structure definitions or in interface blocks are processed.
>      */
>     glsl_struct_field *const fields = ralloc_array(state,
> glsl_struct_field,
>                                                   decl_count);
>
>     unsigned i = 0;
> -   foreach_list_typed (ast_declarator_list, decl_list, link,
> -                      &this->declarations) {
> +   foreach_list_typed (ast_declarator_list, decl_list, link,
> declarations) {
>        const char *type_name;
>
>        decl_list->type->specifier->hir(instructions, state);
> @@ -4051,7 +4052,6 @@ ast_struct_specifier::hir(exec_list *instructions,
>         * embedded structure definitions have been removed from the
> language.
>         */
>        if (state->es_shader && decl_list->type->specifier->structure !=
> NULL) {
> -        YYLTYPE loc = this->get_location();
>          _mesa_glsl_error(&loc, state, "Embedded structure definitions are
> "
>                           "not allowed in GLSL ES 1.00.");
>        }
> @@ -4063,7 +4063,6 @@ ast_struct_specifier::hir(exec_list *instructions,
>                           &decl_list->declarations) {
>          const struct glsl_type *field_type = decl_type;
>          if (decl->is_array) {
> -           YYLTYPE loc = decl->get_location();
>             field_type = process_array_type(&loc, decl_type,
> decl->array_size,
>                                             state);
>          }
> @@ -4076,10 +4075,27 @@ ast_struct_specifier::hir(exec_list *instructions,
>
>     assert(i == decl_count);
>
> +   *fields_ret = fields;
> +   return decl_count;
> +}
> +
> +
> +ir_rvalue *
> +ast_struct_specifier::hir(exec_list *instructions,
> +                         struct _mesa_glsl_parse_state *state)
> +{
> +   YYLTYPE loc = this->get_location();
> +   glsl_struct_field *fields;
> +   unsigned decl_count =
> +      ast_process_structure_or_interface_block(instructions,
> +                                              state,
> +                                              &this->declarations,
> +                                              loc,
> +                                              &fields);
> +
>     const glsl_type *t =
>        glsl_type::get_record_instance(fields, decl_count, this->name);
>
> -   YYLTYPE loc = this->get_location();
>     if (!state->symbols->add_type(name, t)) {
>        _mesa_glsl_error(& loc, state, "struct `%s' previously defined",
> name);
>     } else {
> --
> 1.7.11.7
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130123/ee3630a4/attachment-0001.html>