On 08/13/2013 01:35 PM, Ian Romanick wrote:
From: Ian Romanick <ian.d.roman...@intel.com>
Previously we would emit a warning for empty declarations like
float;
We would also emit the same warning for things like
highp float;
However, this second case is most likely the application trying to set
the default precision. This makes the compiler generate a stronger
warning with some suggestion of a fix.
It really seems like this should be an error. I'll bet that 100% of the
time someone writes 'highp float;' the actually meant 'precision highp
float;'. Alas, both AMD and NVIDIA accept this syntax, and the spec
doesn't explicitly forbid it.
This makes piglit's precision-05.vert generate the following warnings:
0:12(11): warning: empty declaration with precision qualifier, to set the
default precision, use `precision lowp float;'
0:13(12): warning: empty declaration with precision qualifier, to set the
default precision, use `precision mediump int;'
Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
Cc: Kenneth Graunke <kenn...@whitecape.org>
Cc: "9.2" <mesa-sta...@lists.freedesktop.org>
---
src/glsl/ast_to_hir.cpp | 43 ++++++++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 13 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 40992fb..f96b64b 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2719,6 +2719,10 @@ ast_declarator_list::hir(exec_list *instructions,
* name of a known structure type. This is both invalid and weird.
* Emit an error.
*
+ * - The program text contained something like 'mediump float;'
+ * when the programmer probably meant 'precision mediump
+ * float;' Emit an error.
You mean "Emit a warning" here.
+ *
* Note that if decl_type is NULL and there is a structure involved,
* there must have been some sort of error with the structure. In this
* case we assume that an error was already generated on this line of
@@ -2727,20 +2731,33 @@ ast_declarator_list::hir(exec_list *instructions,
*/
assert(this->type->specifier->structure == NULL || decl_type != NULL
|| state->error);
- if (this->type->specifier->structure == NULL) {
- if (decl_type != NULL) {
- _mesa_glsl_warning(&loc, state, "empty declaration");
- } else {
- _mesa_glsl_error(&loc, state,
- "invalid type `%s' in empty declaration",
- type_name);
- }
- }
- if (this->type->qualifier.precision != ast_precision_none &&
- this->type->specifier->structure != NULL) {
- _mesa_glsl_error(&loc, state, "precision qualifiers can't be applied "
- "to structures");
+ if (decl_type == NULL) {
+ _mesa_glsl_error(&loc, state,
+ "invalid type `%s' in empty declaration",
+ type_name);
+ } else if (this->type->qualifier.precision != ast_precision_none) {
+ if (this->type->specifier->structure != NULL)
+ _mesa_glsl_error(&loc, state,
+ "precision qualifiers can't be applied "
+ "to structures");
+ else {
Please use curly braces around the first branch as well. Yes, it's only
one statement, but it's three lines, and adding them doesn't take any
additional space.
Other than those minor nits, this is
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
+ static const char *const precision_names[] = {
+ "highp",
+ "highp",
+ "mediump",
+ "lowp"
+ };
+
+ _mesa_glsl_warning(&loc, state,
+ "empty declaration with precision qualifier, "
+ "to set the default precision, use "
+ "`precision %s %s;'",
+
precision_names[this->type->qualifier.precision],
+ type_name);
+ }
+ } else {
+ _mesa_glsl_warning(&loc, state, "empty declaration");
}
}
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
