Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On Sat, Aug 6, 2011 at 5:35 AM, Ian Romanick i...@freedesktop.org wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 08/05/2011 12:41 AM, Chia-I Wu wrote:
On Fri, Aug 5, 2011 at 1:48 PM, Chad Versace c...@chad-versace.us wrote:
On 08/04/2011 06:18 PM, Chad Versace wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still
present in
the GLSL 4.10 spec, so it's unlikely to be a bug.
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows
empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
Also, please update the commit message to say that `gcc --std=c99` allows
empty
declrations and include the appropriate quotation from the GLSL 1.20 spec's
grammar. Without that extra info, someone may stumble onto this commit and
say WTF.
Oops. I've already committed it...
You have to give a little more time for review. I would like to have
had an opportunity to weigh in, but it's a bit late now.
Yes, I should have given it more time... Sorry about that.
Maybe we can test how the other proprietary drivers work with regard
to this and decide whether to revert it or not? I was told that it is
what the other drivers do on Windows, but I could not verify.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk48VBQACgkQX1gOwKyEAw9ZFACdHy2gnNLdszGp5PbDu84Luay1
tpoAoIcgM2pemUdDINNiAvxc/wWw3J8T
=X0eZ
-END PGP SIGNATURE-
--
o...@lunarg.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On Fri, Aug 5, 2011 at 1:48 PM, Chad Versace c...@chad-versace.us wrote:
On 08/04/2011 06:18 PM, Chad Versace wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present
in
the GLSL 4.10 spec, so it's unlikely to be a bug.
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
Also, please update the commit message to say that `gcc --std=c99` allows
empty
declrations and include the appropriate quotation from the GLSL 1.20 spec's
grammar. Without that extra info, someone may stumble onto this commit and
say WTF.
Oops. I've already committed it...
--
Chad Versace
c...@chad-versace.us
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
o...@lunarg.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On 4 August 2011 18:18, Chad Versace c...@chad-versace.us wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present in
the GLSL 4.10 spec, so it's unlikely to be a bug.
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
--
Chad Versace
c...@chad-versace.us
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Anyone want to volunteer to write a Piglit test for this? It seems
like the kind of obscure corner case that we're likely to regress on
if we don't explicitly test for it.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On 08/05/2011 12:41 AM, Chia-I Wu wrote: On Fri, Aug 5, 2011 at 1:48 PM, Chad Versace c...@chad-versace.us wrote: Also, please update the commit message to say that `gcc --std=c99` allows empty declrations and include the appropriate quotation from the GLSL 1.20 spec's grammar. Without that extra info, someone may stumble onto this commit and say WTF. Oops. I've already committed it... Are you sure? I am unable to find it with `git log origin/master src/glsl`. By the way, I've submitted some tests to the Piglit list. -- Chad Versace c...@chad-versace.us signature.asc Description: OpenPGP digital signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On 08/05/2011 11:45 AM, Paul Berry wrote:
On 4 August 2011 18:18, Chad Versace c...@chad-versace.us wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present
in
the GLSL 4.10 spec, so it's unlikely to be a bug.
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
--
Chad Versace
c...@chad-versace.us
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Anyone want to volunteer to write a Piglit test for this? It seems
like the kind of obscure corner case that we're likely to regress on
if we don't explicitly test for it.
Tests are posted on the Piglit list.
--
Chad Versace
c...@chad-versace.us
signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On 08/04/2011 09:48 PM, Chad Versace wrote:
On 08/04/2011 06:18 PM, Chad Versace wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present
in
the GLSL 4.10 spec, so it's unlikely to be a bug.
You do know that (at least according to Dan M.) they haven't updated the
grammar in the appendix since GLSL 1.50, right? And it already allows
completely bogus things that don't match the rest of the spec (see the
switch statement grammar).
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
Also, please update the commit message to say that `gcc --std=c99` allows
empty
declrations and include the appropriate quotation from the GLSL 1.20 spec's
grammar. Without that extra info, someone may stumble onto this commit and
say WTF.
I still say WTF. Do ATI, nVidia, or Apple support this? If even one of
them doesn't, I say we shouldn't either.
GLSL is so nasty sometimes.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 08/05/2011 12:41 AM, Chia-I Wu wrote:
On Fri, Aug 5, 2011 at 1:48 PM, Chad Versace c...@chad-versace.us wrote:
On 08/04/2011 06:18 PM, Chad Versace wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present
in
the GLSL 4.10 spec, so it's unlikely to be a bug.
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows
empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
Also, please update the commit message to say that `gcc --std=c99` allows
empty
declrations and include the appropriate quotation from the GLSL 1.20 spec's
grammar. Without that extra info, someone may stumble onto this commit and
say WTF.
Oops. I've already committed it...
You have to give a little more time for review. I would like to have
had an opportunity to weigh in, but it's a bit late now.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk48VBQACgkQX1gOwKyEAw9ZFACdHy2gnNLdszGp5PbDu84Luay1
tpoAoIcgM2pemUdDINNiAvxc/wWw3J8T
=X0eZ
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote: From: Chia-I Wu o...@lunarg.com Unlike C++, empty declarations such as float; should be valid. The spec is not explicit about this actually. Some apps that generate their shader sources may rely on this. This was noted when porting one of them to Linux from Windows. Ew. Looking the GLSL 1.20 spec, I see: statement: declaration_statement declaration_statement: declaration declaration: init_declarator_list SEMICOLON init_declarator_list: single_declaration single_declaration: fully_specified_type fully_specified_type IDENTIFIER so it looks like that is actually valid code. That's awful. pgpfJuk23mAKh.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present in
the GLSL 4.10 spec, so it's unlikely to be a bug.
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
--
Chad Versace
c...@chad-versace.us
signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: empty declarations should be valid
On 08/04/2011 06:18 PM, Chad Versace wrote:
On 08/04/2011 01:29 PM, Eric Anholt wrote:
On Thu, 4 Aug 2011 12:59:35 +0900, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
Ew. Looking the GLSL 1.20 spec, I see:
statement:
declaration_statement
declaration_statement:
declaration
declaration:
init_declarator_list SEMICOLON
init_declarator_list:
single_declaration
single_declaration:
fully_specified_type
fully_specified_type IDENTIFIER
so it looks like that is actually valid code. That's awful.
I first suspected that this was a spec grammar bug. But it is still present in
the GLSL 4.10 spec, so it's unlikely to be a bug.
Surprisingly, C also allows empty declarations. Compiling this
float;
int main() { return 0; }
with `gcc --std=c99` succeeds and emits this warning:
warning: useless type name in empty declaration [enabled by default]
I hate to say this, but I believe the spec grammar intentionally allows empty
declarations. C allows it, and GLSL tries to mimic C.
Even though I don't like empty declarations, this patch is
Reviewed-by: Chad Versace c...@chad-versace.us
Also, please update the commit message to say that `gcc --std=c99` allows empty
declrations and include the appropriate quotation from the GLSL 1.20 spec's
grammar. Without that extra info, someone may stumble onto this commit and say
WTF.
--
Chad Versace
c...@chad-versace.us
signature.asc
Description: OpenPGP digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] glsl: empty declarations should be valid
From: Chia-I Wu o...@lunarg.com
Unlike C++, empty declarations such as
float;
should be valid. The spec is not explicit about this actually.
Some apps that generate their shader sources may rely on this. This was
noted when porting one of them to Linux from Windows.
---
src/glsl/ast_to_hir.cpp | 10 +-
src/glsl/glsl_parser.yy | 10 +++---
2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index c0524bf..7da1461 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2399,12 +2399,12 @@ ast_declarator_list::hir(exec_list *instructions,
decl_type = this-type-specifier-glsl_type( type_name, state);
if (this-declarations.is_empty()) {
- /* The only valid case where the declaration list can be empty is when
- * the declaration is setting the default precision of a built-in type
- * (e.g., 'precision highp vec4;').
- */
-
if (decl_type != NULL) {
+/* Warn if this empty declaration is not for declaring a structure.
+ */
+if (this-type-specifier-structure == NULL) {
+ _mesa_glsl_warning(loc, state, empty declaration);
+}
} else {
_mesa_glsl_error( loc, state, incomplete declaration);
}
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 2c0498e..1851f1e 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -971,13 +971,9 @@ single_declaration:
fully_specified_type
{
void *ctx = state;
- if ($1-specifier-type_specifier != ast_struct) {
- _mesa_glsl_error( @1, state, empty declaration list\n);
- YYERROR;
- } else {
- $$ = new(ctx) ast_declarator_list($1);
- $$-set_location(yylloc);
- }
+ /* Empty declaration list is valid. */
+ $$ = new(ctx) ast_declarator_list($1);
+ $$-set_location(yylloc);
}
| fully_specified_type any_identifier
{
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
