[Mesa-dev] [PATCH 2/2] glsl: Prefer unreachable(condition) over assert(!condition)
The unreachable macro has the advantage (for modern compilers) of
hinting to the compiler that this code is actually unreachable, which
can help reduce spurious warnings, etc.
Also, this version is a bit easier to type correctly and understand
when reading without that seemingly out-of-place logical negation.
These were all found with the following:
git grep 'assert(! *' -- src/glsl
and then replaced automatically.
---
I did this only for for the glsl directory for now. There are still many more
of these throughout mesa's source tree, but it felt like it would be far too
invasive to make a change like this globally.
So most of the changes here are in the glsl_types.cpp file where I was already
working, and then there are just small numbers in many of the files in the
same directory.
src/glsl/ast_function.cpp| 4 +--
src/glsl/ast_to_hir.cpp | 8 ++---
src/glsl/glcpp/glcpp-parse.y | 2 +-
src/glsl/glsl_parser_extras.cpp | 4 +--
src/glsl/glsl_symbol_table.cpp | 4 +--
src/glsl/glsl_types.cpp | 14 -
src/glsl/ir.cpp | 38
src/glsl/ir.h| 2 +-
src/glsl/ir_clone.cpp| 2 +-
src/glsl/ir_constant_expression.cpp | 8 ++---
src/glsl/ir_equals.cpp | 2 +-
src/glsl/ir_set_program_inouts.cpp | 2 +-
src/glsl/ir_validate.cpp | 2 +-
src/glsl/ir_visitor.h| 2 +-
src/glsl/link_interface_blocks.cpp | 2 +-
src/glsl/link_uniform_block_active_visitor.cpp | 2 +-
src/glsl/link_uniform_blocks.cpp | 2 +-
src/glsl/link_uniform_initializers.cpp | 4 +--
src/glsl/link_uniforms.cpp | 2 +-
src/glsl/link_varyings.cpp | 4 +--
src/glsl/loop_analysis.cpp | 2 +-
src/glsl/loop_controls.cpp | 2 +-
src/glsl/lower_packed_varyings.cpp | 4 +--
src/glsl/lower_packing_builtins.cpp | 2 +-
src/glsl/lower_ubo_reference.cpp | 6 ++--
src/glsl/lower_variable_index_to_cond_assign.cpp | 2 +-
src/glsl/lower_vector.cpp| 2 +-
src/glsl/opt_constant_propagation.cpp| 4 +--
src/glsl/opt_minmax.cpp | 2 +-
29 files changed, 68 insertions(+), 68 deletions(-)
diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index cbff9d8..60a5414 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -661,7 +661,7 @@ dereference_component(ir_rvalue *src, unsigned component)
return dereference_component(col, r);
}
- assert(!Should not get here.);
+ unreachable(Should not get here.);
return NULL;
}
@@ -1016,7 +1016,7 @@ emit_inline_vector_constructor(const glsl_type *type,
data.b[i + base_component] = c-get_bool_component(i);
break;
default:
- assert(!Should not get here.);
+ unreachable(Should not get here.);
break;
}
}
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 811a955..ae68142 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -1193,7 +1193,7 @@ ast_expression::do_hir(exec_list *instructions,
switch (this-oper) {
case ast_aggregate:
- assert(!ast_aggregate: Should never get here.);
+ unreachable(ast_aggregate: Should never get here.);
break;
case ast_assign: {
@@ -2314,7 +2314,7 @@ validate_explicit_location(const struct
ast_type_qualifier *qual,
: (qual-location + VARYING_SLOT_VAR0);
break;
case MESA_SHADER_COMPUTE:
-assert(!Unexpected shader type);
+unreachable(Unexpected shader type);
break;
}
} else {
@@ -5412,7 +5412,7 @@ ast_interface_block::hir(exec_list *instructions,
} else {
var_mode = ir_var_auto;
iface_type_name = UNKNOWN;
- assert(!interface block layout qualifier not found!);
+ unreachable(interface block layout qualifier not found!);
}
enum glsl_matrix_layout matrix_layout = GLSL_MATRIX_LAYOUT_INHERITED;
@@ -6008,7 +6008,7 @@ remove_per_vertex_blocks(exec_list *instructions,
}
break;
default:
- assert(!Unexpected mode);
+ unreachable(Unexpected mode);
break;
}
diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
index f1119eb..f1c006e 100644
--- a/src/glsl/glcpp/glcpp-parse.y
+++ b/src/glsl/glcpp/glcpp-parse.y
@@ -1169,7 +1169,7 @@ _token_print (char **out, size_t *len, token_t *token)
/* Nothing to print. */
break;
default:
-
Re: [Mesa-dev] [PATCH 2/2] glsl: Prefer unreachable(condition) over assert(!condition)
On 12/05/2014 01:17 PM, Carl Worth wrote:
The unreachable macro has the advantage (for modern compilers) of
hinting to the compiler that this code is actually unreachable, which
can help reduce spurious warnings, etc.
Also, this version is a bit easier to type correctly and understand
when reading without that seemingly out-of-place logical negation.
These were all found with the following:
git grep 'assert(! *' -- src/glsl
and then replaced automatically.
There are a bunch of remove this ... comments below. I also think the
Should not get here comments should get replace with something more
reasonable. Most distros build without -DNDEBUG, so I tried to use the
same string over and over to keep the bloat down. If the strings only
exist in debug builds, there's no reason to be stingy.
There are also a couple spots that I think should remain asserts. I've
commented on those below.
---
I did this only for for the glsl directory for now. There are still many more
of these throughout mesa's source tree, but it felt like it would be far too
invasive to make a change like this globally.
So most of the changes here are in the glsl_types.cpp file where I was already
working, and then there are just small numbers in many of the files in the
same directory.
src/glsl/ast_function.cpp| 4 +--
src/glsl/ast_to_hir.cpp | 8 ++---
src/glsl/glcpp/glcpp-parse.y | 2 +-
src/glsl/glsl_parser_extras.cpp | 4 +--
src/glsl/glsl_symbol_table.cpp | 4 +--
src/glsl/glsl_types.cpp | 14 -
src/glsl/ir.cpp | 38
src/glsl/ir.h| 2 +-
src/glsl/ir_clone.cpp| 2 +-
src/glsl/ir_constant_expression.cpp | 8 ++---
src/glsl/ir_equals.cpp | 2 +-
src/glsl/ir_set_program_inouts.cpp | 2 +-
src/glsl/ir_validate.cpp | 2 +-
src/glsl/ir_visitor.h| 2 +-
src/glsl/link_interface_blocks.cpp | 2 +-
src/glsl/link_uniform_block_active_visitor.cpp | 2 +-
src/glsl/link_uniform_blocks.cpp | 2 +-
src/glsl/link_uniform_initializers.cpp | 4 +--
src/glsl/link_uniforms.cpp | 2 +-
src/glsl/link_varyings.cpp | 4 +--
src/glsl/loop_analysis.cpp | 2 +-
src/glsl/loop_controls.cpp | 2 +-
src/glsl/lower_packed_varyings.cpp | 4 +--
src/glsl/lower_packing_builtins.cpp | 2 +-
src/glsl/lower_ubo_reference.cpp | 6 ++--
src/glsl/lower_variable_index_to_cond_assign.cpp | 2 +-
src/glsl/lower_vector.cpp| 2 +-
src/glsl/opt_constant_propagation.cpp| 4 +--
src/glsl/opt_minmax.cpp | 2 +-
29 files changed, 68 insertions(+), 68 deletions(-)
diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index cbff9d8..60a5414 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -661,7 +661,7 @@ dereference_component(ir_rvalue *src, unsigned component)
return dereference_component(col, r);
}
- assert(!Should not get here.);
+ unreachable(Should not get here.);
return NULL;
Remove this return.
}
@@ -1016,7 +1016,7 @@ emit_inline_vector_constructor(const glsl_type *type,
data.b[i + base_component] = c-get_bool_component(i);
break;
default:
- assert(!Should not get here.);
+ unreachable(Should not get here.);
break;
}
}
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 811a955..ae68142 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -1193,7 +1193,7 @@ ast_expression::do_hir(exec_list *instructions,
switch (this-oper) {
case ast_aggregate:
- assert(!ast_aggregate: Should never get here.);
+ unreachable(ast_aggregate: Should never get here.);
break;
case ast_assign: {
@@ -2314,7 +2314,7 @@ validate_explicit_location(const struct
ast_type_qualifier *qual,
: (qual-location + VARYING_SLOT_VAR0);
break;
case MESA_SHADER_COMPUTE:
-assert(!Unexpected shader type);
+unreachable(Unexpected shader type);
break;
}
} else {
@@ -5412,7 +5412,7 @@ ast_interface_block::hir(exec_list *instructions,
} else {
var_mode = ir_var_auto;
iface_type_name = UNKNOWN;
- assert(!interface block layout qualifier not found!);
+ unreachable(interface block layout qualifier not found!);
}
enum
