Bug 101336

Summary: glcpp-test.sh regression
Product: Mesa Reporter: Vinson Lee <vlee>
Component: Mesa coreAssignee: mesa-dev
Status: RESOLVED FIXED QA Contact: mesa-dev
Severity: normal    
Priority: medium CC: itoral, samuel.pitoiset
Version: gitKeywords: bisected, regression
Hardware: x86-64 (AMD64)   
OS: All   
Whiteboard:
i915 platform: i915 features:

Description Vinson Lee 2017-06-07 20:35:32 UTC
mesa: c705caaff9e6cc874e90651d6f8e459741978b06 (master 17.2.0-devel)

Testing 120-undef-builtin.c... FAIL
--- ./glsl/glcpp/tests/120-undef-builtin.c.expected     2016-08-17 10:35:41.821191603 -0700
+++ src/compiler/glsl/glcpp/tests/120-undef-builtin.c.out  2017-06-07 13:32:22.797950382 -0700
@@ -1,6 +1,6 @@
-0:2(1): preprocessor error: Built-in (pre-defined) macro names cannot be undefined.
-0:3(1): preprocessor error: Built-in (pre-defined) macro names cannot be undefined.
-0:4(1): preprocessor error: Built-in (pre-defined) macro names cannot be undefined.
+0:2(1): preprocessor error: Built-in (pre-defined) names cannot be undefined.
+0:3(1): preprocessor error: Built-in (pre-defined) names cannot be undefined.
+0:4(1): preprocessor error: Built-in (pre-defined) names cannot be undefined.
 #version 300 es
Comment 1 Vinson Lee 2017-06-07 21:48:00 UTC
f7741985be0234c3fe71e1f97740579e35726b92 is the first bad commit
commit f7741985be0234c3fe71e1f97740579e35726b92
Author: Iago Toral Quiroga <itoral@igalia.com>
Date:   Tue May 30 13:25:35 2017 +0200

    glcpp: fix #undef to match latest spec update and GLSLang implementation
    
    GLSL ES spec includes the following:
    
       "It is an error to undefine or to redefine a built-in
        (pre-defined) macro name."
    
    But desktop GLSL doesn't. This has sparked some discussion
    in Khronos, and the final conclusion was to update the
    GLSL 4.50 spec to include the following:
    
       "By convention, all macro names containing two consecutive
        underscores ( __ ) are reserved for use by underlying
        software layers.  Defining or undefining such a name in a
        shader does not itself result in an error, but may result
        in unintended behaviors that stem from having multiple
        definitions of the same name.  All macro names prefixed
        with “GL_” (“GL” followed by a single underscore) are also
        reserved, and defining or undefining such a name results in
        a compile-time error."
    
    In other words, undefining GL_* names should be an error, but
    undefining other names with a double underscore in them is
    not strictly prohibited in desktop GLSL.
    
    This patch fixes the preprocessor to apply these rules,
    following exactly the implementation already present
    in GLSLang. This fixes some tests in CTS.
    
    Khronos bug:
    https://cvs.khronos.org/bugzilla/show_bug.cgi?id=16003
    
    Fixes:
    KHR-GL45.shaders.preprocessor.definitions.undefine_core_profile_vertex
    KHR-GL45.shaders.preprocessor.definitions.undefine_core_profile_fragment
    
    Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>

:040000 040000 fc6e2499a69db492e708bba90dfb2958a2653f81 525845c2b0a66996639043ef0bb88b5945db2964 M	src
bisect run success
Comment 2 Iago Toral 2017-06-08 06:27:57 UTC
Yes, my commit modified slightly the error messages, I'll send a patch with to update the test to the new output. Thanks for letting me know.
Comment 3 Iago Toral 2017-06-08 06:43:38 UTC
Link to patch:
https://lists.freedesktop.org/archives/mesa-dev/2017-June/158361.html
Comment 4 Iago Toral 2017-06-08 07:56:27 UTC
Fixed in master with:

commit ce53e8e61b9034ddf124a834a7d706c8a0e808ec
Author: Iago Toral Quiroga <itoral@igalia.com>
Date:   Thu Jun 8 08:38:29 2017 +0200

    Fix glcpp test expectations
    
    With commit f7741985be0234 we have changed some preprocessor
    error messages and warnings. Adapt related glcpp tests
    expectations accordingly.
    
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101336
    Tested-by: Vinson Lee <vlee@freedesktop.org>
    Reviewed-by: Samuel Pitoiset <samuel.pitoiset@gmail.com>

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.