[Mesa-dev] [PATCH 4/9] glsl: report errors via GL_ARB_debug_output

[Ian Romanick]

On 04/13/2012 08:51 AM, nobled wrote:
> ---
>   src/glsl/glsl_parser_extras.cpp     |   14 ++++++++++++++
>   src/glsl/standalone_scaffolding.cpp |    6 ++++++
>   src/glsl/standalone_scaffolding.h   |    4 ++++
>   3 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index aade2e1..d6a156d 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -139,13 +139,27 @@ static void
>   _mesa_glsl_msg(const YYLTYPE *locp, _mesa_glsl_parse_state *state,
>                  bool error, const char *fmt, va_list ap)
>   {
> +   struct gl_context *ctx = state->ctx;

Since this is C++ code, I'd suggest moving the declarations down to the 
assignments.

> +   int msg_offset;
> +   const char *msg;
> +
>      assert(state->info_log != NULL);
> +
> +   /* Get the offset that the new message will be written to. */
> +   msg_offset = strlen(state->info_log);
> +
>      ralloc_asprintf_append(&state->info_log, "%u:%u(%u): %s: ",
>   					    locp->source,
>   					    locp->first_line,
>   					    locp->first_column,
>   					    error ? "error" : "warning");
>      ralloc_vasprintf_append(&state->info_log, fmt, ap);
> +
> +   msg =&state->info_log[msg_offset];

So this would become

     const char *const msg = &state->info_log[msg_offset];

> +   /* Report the error via GL_ARB_debug_output. */
> +   if (error)
> +      _mesa_shader_error(ctx, SHADER_ERROR_UNKNOWN, msg, strlen(msg));

Should warnings also be reported?  Maybe use GL_DEBUG_TYPE_OTHER_ARB? 
We don't report that many warnings, so it may not matter that much. 
Same comment in patch 9/9 for the preprocessor warnings.

> +
>      ralloc_strcat(&state->info_log, "\n");
>   }
>
> diff --git a/src/glsl/standalone_scaffolding.cpp
> b/src/glsl/standalone_scaffolding.cpp
> index 24cc64a..99079b1 100644
> --- a/src/glsl/standalone_scaffolding.cpp
> +++ b/src/glsl/standalone_scaffolding.cpp
> @@ -41,6 +41,12 @@ _mesa_reference_shader(struct gl_context *ctx,
> struct gl_shader **ptr,
>      *ptr = sh;
>   }
>
> +void
> +_mesa_shader_error(struct gl_context *, gl_shader_error,
> +                   const char *, int)

Is that valid C++?  I thought you could only leave the names out in the 
prototype.  This makes it a bit odd that the names in included in the 
prototype (in standalone_scaffolding.h, below).

> +{
> +}
> +
>   struct gl_shader *
>   _mesa_new_shader(struct gl_context *ctx, GLuint name, GLenum type)
>   {
> diff --git a/src/glsl/standalone_scaffolding.h
> b/src/glsl/standalone_scaffolding.h
> index 8773320..8dedbaa 100644
> --- a/src/glsl/standalone_scaffolding.h
> +++ b/src/glsl/standalone_scaffolding.h
> @@ -40,6 +40,10 @@ _mesa_reference_shader(struct gl_context *ctx,
> struct gl_shader **ptr,
>   extern "C" struct gl_shader *
>   _mesa_new_shader(struct gl_context *ctx, GLuint name, GLenum type);
>
> +extern "C" void
> +_mesa_shader_error(struct gl_context *ctx, gl_shader_error err,
> +                   const char *msg, int len);
> +
>   /**
>    * Initialize the given gl_context structure to a reasonable set of
>    * defaults representing the minimum capabilities required by the