On 22 January 2013 00:51, Ian Romanick <i...@freedesktop.org> wrote:
> From: Ian Romanick <ian.d.roman...@intel.com>
>
> This will soon also be used for processing interface block fields.
>
> Signed-off-by: Ian Romanick <ian.d.roman...@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 <stereotype...@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
