[Mesa-dev] [PATCH] glsl: Restore Mesa-style to shader_enums.c/h.

[Ian Romanick]

On 01/15/2016 01:44 PM, Matt Turner wrote:
> On Fri, Jan 15, 2016 at 1:39 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> On Fri, Jan 15, 2016 at 4:34 PM, Matt Turner <mattst88 at gmail.com> wrote:
>>> ---
>>>  src/glsl/nir/shader_enums.c | 24 ++++++++++++++++--------
>>>  src/glsl/nir/shader_enums.h | 16 ++++++++--------
>>>  2 files changed, 24 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/src/glsl/nir/shader_enums.c b/src/glsl/nir/shader_enums.c
>>> index 10f546a..16b20db 100644
>>> --- a/src/glsl/nir/shader_enums.c
>>> +++ b/src/glsl/nir/shader_enums.c
>>> @@ -33,7 +33,8 @@
>>>  #define ENUM(x) [x] = #x
>>>  #define NAME(val) ((((val) < ARRAY_SIZE(names)) && names[(val)]) ? names[(val)] : "UNKNOWN")
>>>
>>> -const char * gl_shader_stage_name(gl_shader_stage stage)
>>> +const char *
>>> +gl_shader_stage_name(gl_shader_stage stage)
>>>  {
>>>     static const char *names[] = {
>>>        ENUM(MESA_SHADER_VERTEX),
>>> @@ -51,7 +52,8 @@ const char * gl_shader_stage_name(gl_shader_stage stage)
>>>   * Translate a gl_shader_stage to a short shader stage name for debug
>>>   * printouts and error messages.
>>>   */
>>> -const char * _mesa_shader_stage_to_string(unsigned stage)
>>> +const char *
>>> +_mesa_shader_stage_to_string(unsigned stage)
>>>  {
>>>     switch (stage) {
>>>     case MESA_SHADER_VERTEX:   return "vertex";
>>> @@ -69,7 +71,8 @@ const char * _mesa_shader_stage_to_string(unsigned stage)
>>>   * Translate a gl_shader_stage to a shader stage abbreviation (VS, GS, FS)
>>>   * for debug printouts and error messages.
>>>   */
>>> -const char * _mesa_shader_stage_to_abbrev(unsigned stage)
>>> +const char *
>>> +_mesa_shader_stage_to_abbrev(unsigned stage)
>>>  {
>>>     switch (stage) {
>>>     case MESA_SHADER_VERTEX:   return "VS";
>>> @@ -83,7 +86,8 @@ const char * _mesa_shader_stage_to_abbrev(unsigned stage)
>>>     unreachable("Unknown shader stage.");
>>>  }
>>>
>>> -const char * gl_vert_attrib_name(gl_vert_attrib attrib)
>>> +const char *
>>> +gl_vert_attrib_name(gl_vert_attrib attrib)
>>>  {
>>>     static const char *names[] = {
>>>        ENUM(VERT_ATTRIB_POS),
>>> @@ -124,7 +128,8 @@ const char * gl_vert_attrib_name(gl_vert_attrib attrib)
>>>     return NAME(attrib);
>>>  }
>>>
>>> -const char * gl_varying_slot_name(gl_varying_slot slot)
>>> +const char *
>>> +gl_varying_slot_name(gl_varying_slot slot)
>>>  {
>>>     static const char *names[] = {
>>>        ENUM(VARYING_SLOT_POS),
>>> @@ -190,7 +195,8 @@ const char * gl_varying_slot_name(gl_varying_slot slot)
>>>     return NAME(slot);
>>>  }
>>>
>>> -const char * gl_system_value_name(gl_system_value sysval)
>>> +const char *
>>> +gl_system_value_name(gl_system_value sysval)
>>>  {
>>>     static const char *names[] = {
>>>       ENUM(SYSTEM_VALUE_VERTEX_ID),
>>> @@ -216,7 +222,8 @@ const char * gl_system_value_name(gl_system_value sysval)
>>>     return NAME(sysval);
>>>  }
>>>
>>> -const char * glsl_interp_qualifier_name(enum glsl_interp_qualifier qual)
>>> +const char *
>>> +glsl_interp_qualifier_name(enum glsl_interp_qualifier qual)
>>>  {
>>>     static const char *names[] = {
>>>        ENUM(INTERP_QUALIFIER_NONE),
>>> @@ -228,7 +235,8 @@ const char * glsl_interp_qualifier_name(enum glsl_interp_qualifier qual)
>>>     return NAME(qual);
>>>  }
>>>
>>> -const char * gl_frag_result_name(gl_frag_result result)
>>> +const char *
>>> +gl_frag_result_name(gl_frag_result result)
>>>  {
>>>     static const char *names[] = {
>>>        ENUM(FRAG_RESULT_DEPTH),
>>> diff --git a/src/glsl/nir/shader_enums.h b/src/glsl/nir/shader_enums.h
>>> index c747464..efc0b0d 100644
>>> --- a/src/glsl/nir/shader_enums.h
>>> +++ b/src/glsl/nir/shader_enums.h
>>> @@ -47,19 +47,19 @@ typedef enum
>>>     MESA_SHADER_COMPUTE = 5,
>>>  } gl_shader_stage;
>>>
>>> -const char * gl_shader_stage_name(gl_shader_stage stage);
>>> +const char *gl_shader_stage_name(gl_shader_stage stage);
>>
>> Wouldn't the mesa style be to do
>>
>> const char *
>> gl_shader_stage_name(gl_shader_stage stage);
> 
> It seems we do that in headers sometimes, but the real value in
> separating the return type from the function name is to be able to
> grep '^func' to find the definition in the *.c file. I'm fine with
> prototypes in headers having the return type on the same line.

It also helps you find where the prototype is.  If I want to add a
parameter to a function, I need to find and update both places.  It
doesn't matter as much when the .c and .h have the same name, but for
cases like meta.h or brw_context.h (which doesn't completely follow this
style), it matters more.

>> ? (Along with "extern" thrown in for no apparent reason.)
> 
> Heh :)
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>