[Mesa-dev] Should we add new error cases to stable releases?

[Carl Worth]

Hi Ken,

In the last stable-release cycle you argued for not cherry picking
commit fcaa48d9cc8937e0ceb59dfd22ef5b6e6fd1a273 on the grounds that
we shouldn't change mesa to be strictly more strict in a stable release.

That is, adding a new error case to the compiler cannot actually cause a
valid program to start working, (which would be a nice bug fix), but
could potentially cause an invalid program to stop working (which could
be regarded as a regression).

The same reasoning seems to disqualify the below patch. Do you agree?

Anyone else have input on this issue? If we agree on the rationale, I'd
like to codify it with some language in our documentation for
stable-branch criteria. Something like:

	Note: Not all bug fixes are automatically suitable for the
	stable branch.

	For example, a patch that makes the implementation more strict,
	(such as detecting some invalid GLSL code that was previously
	silently accepted), can be rejected by the stable-branch
	maintainer. The rationale is that a patch like this can cause a
	program which was previously working as desired to now stop
	working due to the new error case. From the point of view of the
	user of such an application, that's a regression that we do not
	want to allow during a stable release.

-Carl

commit 17856726c94000bf16156f7f9acea77a271a6005
Author: Kenneth Graunke <kenneth at whitecape.org>
Date:   Fri Jul 26 21:18:56 2013 -0700

    glsl: Disallow auxiliary storage qualifiers on FS outputs.
    
    This has always been an error; we just forgot to check for it.
    
    Fixes Piglit's no-aux-qual-on-fs-output.frag.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=67333
    Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
    Reviewed-by: Matt Turner <mattst88 at gmail.com>
    Cc: mesa-stable at lists.freedesktop.org

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 2569cde..598da92 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2927,6 +2927,20 @@ ast_declarator_list::hir(exec_list *instructions,
                           "'centroid in' cannot be used in a vertex shader");
       }
 
+      /* Section 4.3.6 of the GLSL 1.30 specification states:
+       * "It is an error to use centroid out in a fragment shader."
+       *
+       * The GL_ARB_shading_language_420pack extension specification states:
+       * "It is an error to use auxiliary storage qualifiers or interpolation
+       *  qualifiers on an output in a fragment shader."
+       */
+      if (state->target == fragment_shader &&
+          this->type->qualifier.flags.q.out &&
+          this->type->qualifier.has_auxiliary_storage()) {
+         _mesa_glsl_error(&loc, state,
+                          "auxiliary storage qualifiers cannot be used on "
+                          "fragment shader outputs");
+      }
 
       /* Precision qualifiers exists only in GLSL versions 1.00 and >= 1.30.
        */
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130730/4e1a234e/attachment.pgp>