[Mesa-dev] [PATCH] ast: Updated AST_NUM_OPERATORS for coherence with ast_operators

[Andres Gomez]

On Tue, 2016-08-02 at 17:04 +0100, Eric Engestrom wrote:
> On Tue, Aug 02, 2016 at 12:20:54PM +0300, Andres Gomez wrote:

[snip]

> > I'm not sure I'm understanding what you mean.
> > 
> > If you mean to remove the #define and add the value as and additional
> > element to the enum, the existence of the define is precisely for not
> > doing that, as explained in its comment.
> 
> I've read the comment, so this isn't what I was suggesting (I learned
> something btw, it hadn't occurred to me adding the counter to the enum
> would cause a warning, but I guess it makes sense).
> 
> > 
> > If you mean to move the #define just below the enumeration, that's what
> > this patch does.
> 
> What I meant is to move it to the end of the enum, not after it, so that
> it becomes explicit that it's part of it (for humans, that is; it makes
> no difference to the preprocessor). I expect it was originally just
> after the enum, but with time things got inserted in between.
> 
> Something like this would be better IMHO:
> 
> 8<--------------
> 
> diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
> index 157895d..b5277fc 100644
> --- a/src/compiler/glsl/ast.h
> +++ b/src/compiler/glsl/ast.h
> @@ -198,6 +198,15 @@ enum ast_operators {
>  
>     ast_sequence,
>     ast_aggregate
> +
> +   /**
> +    * Number of possible operators for an ast_expression
> +    *
> +    * This is done as a define instead of as an additional value in the enum so
> +    * that the compiler won't generate spurious messages like "warning:
> +    * enumeration value ‘ast_num_operators’ not handled in switch"
> +    */
> +   #define AST_NUM_OPERATORS (ast_aggregate + 1)
>  };
>  
>  /**
> 
> 8<--------------
> 
> It's not all that important, your change is good either way, so:
> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

Ouch, right!

Thanks for review and extended comments. I will apply your changes and
push.

Thanks!

-- 

Br,

Andres