[Mesa-dev] [PATCH] glsl: Fix function return typechecking

[Oscar Blumberg]

Hi,

On Thu, Feb 21, 2019 at 10:35 AM Tapani Pälli <tapani.palli at intel.com>
wrote:

> Hi;
>
> On 2/11/19 6:46 PM, Oscar Blumberg wrote:
> > apply_implicit_conversion only converts and check base types but we
> > need actual type equality for function returns, otherwise you can
> > return a vec2 from a function declared as returning a float.
>
> Do you have some test shader that hits this condition? It seems to me
> that currently we will error out correctly if one tries to return vec2
> from function declared as returning a float.
>
>
Yes, I believe it triggers in even the simplest case here, such as :
float f() { return vec2(1.0); }
anywhere in the shader.
Does it fail to reproduce on your end ?
(note that the declared glsl version must be recent enough that implicit
conversion for function returns are enabled, I'm using 450 here).

I believe most of the confusion comes from the name of the
apply_implicit_conversion function, since it is mainly used for arithmetic
operations for which, e.g., vec2+float is allowed.
Because of that it only checks (and convert in place) base types without
looking at element count for vector-like things.
We can still use it to perform the conversion but it requires an additional
check, hence the patch.

That's my understanding of it anyway.

> ---
> >   src/compiler/glsl/ast_to_hir.cpp | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/compiler/glsl/ast_to_hir.cpp
> b/src/compiler/glsl/ast_to_hir.cpp
> > index 620153e6a34..6bf2910954f 100644
> > --- a/src/compiler/glsl/ast_to_hir.cpp
> > +++ b/src/compiler/glsl/ast_to_hir.cpp
> > @@ -6248,7 +6248,8 @@ ast_jump_statement::hir(exec_list *instructions,
> >
> >               if (state->has_420pack()) {
> >                  if
> (!apply_implicit_conversion(state->current_function->return_type,
> > -                                              ret, state)) {
> > +                                              ret, state)
> > +                   || (ret->type !=
> state->current_function->return_type)) {
> >                     _mesa_glsl_error(& loc, state,
> >                                      "could not implicitly convert
> return value "
> >                                      "to %s, in function `%s'",
> >
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190221/808ce8b0/attachment-0001.html>