On 26 July 2011 16:08, Chad Versace <c...@chad-versace.us> wrote:
> The parameters were named 'a' and 'b', but there was no documentation
> indicating which was being converted to which. So rename 'a' -> 'to' and
> 'b' -> 'from'.
>
> Also add a Doxygen comment.
>
> Signed-off-by: Chad Versace <c...@chad-versace.us>
> ---
> src/glsl/ir_function.cpp | 27 +++++++++++++++++----------
> 1 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/src/glsl/ir_function.cpp b/src/glsl/ir_function.cpp
> index 6795988..e6ba359 100644
> --- a/src/glsl/ir_function.cpp
> +++ b/src/glsl/ir_function.cpp
> @@ -24,29 +24,36 @@
> #include "glsl_types.h"
> #include "ir.h"
>
> +/**
> + * \brief Check if an implicit type conversion is possible.
> + *
> + * Called by type_compare().
> + *
> + * \return If conversion is possible, 1. Else, -1.
Actually, if I'm not mistaken, it's:
0 if the types match exactly.
1 if the types don't match, but a conversion is possible.
-1 if the types don't match, and a conversion is impossible.
> + */
> int
> -type_compare(const glsl_type *a, const glsl_type *b)
> +type_compare(const glsl_type *to, const glsl_type *from)
> {
> /* If the types are the same, they trivially match.
> */
> - if (a == b)
> + if (to == from)
> return 0;
>
> - switch (a->base_type) {
> + switch (to->base_type) {
> case GLSL_TYPE_UINT:
> case GLSL_TYPE_INT:
> case GLSL_TYPE_BOOL:
> /* There is no implicit conversion to or from integer types or bool.
> */
> - if ((a->is_integer() != b->is_integer())
> - || (a->is_boolean() != b->is_boolean()))
> + if ((to->is_integer() != from->is_integer())
> + || (to->is_boolean() != from->is_boolean()))
> return -1;
>
> /* FALLTHROUGH */
>
> case GLSL_TYPE_FLOAT:
> - if ((a->vector_elements != b->vector_elements)
> - || (a->matrix_columns != b->matrix_columns))
> + if ((to->vector_elements != from->vector_elements)
> + || (to->matrix_columns != from->matrix_columns))
> return -1;
>
> return 1;
> @@ -58,8 +65,8 @@ type_compare(const glsl_type *a, const glsl_type *b)
> return -1;
>
> case GLSL_TYPE_ARRAY:
> - if ((b->base_type != GLSL_TYPE_ARRAY)
> - || (a->length != b->length))
> + if ((from->base_type != GLSL_TYPE_ARRAY)
> + || (to->length != from->length))
> return -1;
>
> /* From GLSL 1.50 spec, page 27 (page 33 of the PDF):
> @@ -68,7 +75,7 @@ type_compare(const glsl_type *a, const glsl_type *b)
> * If the comparison of the array element types detects that a
> conversion
> * would be required, the array types do not match.
> */
> - return (type_compare(a->fields.array, b->fields.array) == 0) ? 0 : -1;
> + return (type_compare(to->fields.array, from->fields.array) == 0) ? 0 :
> -1;
>
> case GLSL_TYPE_VOID:
> case GLSL_TYPE_ERROR:
> --
> 1.7.6
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
I realize this is a bit of yak shaving, but I'm not thrilled about
type_compare's three possible return values. I'd prefer to move
responsibility for checking whether the types match exactly into the
caller. Then type_compare would only need two return values, so it
could return a boolean, and we could name the function in such a way
that the meaning of the boolean was obvious. For example:
bool is_conversion_possible(const glsl_type *to, const glsl_type *from);
Any chance we could do this bit of clean-up while we're in the neighborhood?
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
