[Mesa-dev] [PATCH 1/3] glcpp: Only warn for macro names containing __

[Darius Spitznagel]

Am 19.02.2014 20:09, schrieb Ian Romanick:
> I'm hoping that Tapani or Darius will verify that this patch actually
> fixes the problem.  That's why people CC other people on patches. :)
>
> On 02/18/2014 10:19 AM, Ian Romanick wrote:
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Section 3.3 (Preprocessor) of the GLSL 1.30 spec (and later) and the
>> GLSL ES spec (all versions) say:
>>
>>      "All macro names containing two consecutive underscores ( __ ) are
>>      reserved for future use as predefined macro names. All macro names
>>      prefixed with "GL_" ("GL" followed by a single underscore) are also
>>      reserved."
>>
>> The intention is that names containing __ are reserved for internal use
>> by the implementation, and names prefixed with GL_ are reserved for use
>> by Khronos.  Since every extension adds a name prefixed with GL_ (i.e.,
>> the name of the extension), that should be an error.  Names simply
>> containing __ are dangerous to use, but should be allowed.  In similar
>> cases, the C++ preprocessor specification says, "no diagnostic is
>> required."
>>
>> Per the Khronos bug mentioned below, a future version of the GLSL
>> specification will clarify this.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> Cc: "9.2 10.0 10.1" <mesa-stable at lists.freedesktop.org>
>> Cc: Tapani Pälli <lemody at gmail.com>
>> Cc: Kenneth Graunke <kenneth at whitecape.org>
>> Cc: Darius Spitznagel <d.spitznagel at goodbytez.de>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71870
>> Bugzilla: Khronos #11702
>> ---
>>   src/glsl/glcpp/glcpp-parse.y                       | 22 +++++++++++++++++++---
>>   .../tests/086-reserved-macro-names.c.expected      |  4 ++--
>>   2 files changed, 21 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
>> index 5bb2891..bdc598f 100644
>> --- a/src/glsl/glcpp/glcpp-parse.y
>> +++ b/src/glsl/glcpp/glcpp-parse.y
>> @@ -1770,11 +1770,27 @@ static void
>>   _check_for_reserved_macro_name (glcpp_parser_t *parser, YYLTYPE *loc,
>>   				const char *identifier)
>>   {
>> -	/* According to the GLSL specification, macro names starting with "__"
>> -	 * or "GL_" are reserved for future use.  So, don't allow them.
>> +	/* Section 3.3 (Preprocessor) of the GLSL 1.30 spec (and later) and
>> +	 * the GLSL ES spec (all versions) say:
>> +	 *
>> +	 *     "All macro names containing two consecutive underscores ( __ )
>> +	 *     are reserved for future use as predefined macro names. All
>> +	 *     macro names prefixed with "GL_" ("GL" followed by a single
>> +	 *     underscore) are also reserved."
>> +	 *
>> +	 * The intention is that names containing __ are reserved for internal
>> +	 * use by the implementation, and names prefixed with GL_ are reserved
>> +	 * for use by Khronos.  Since every extension adds a name prefixed
>> +	 * with GL_ (i.e., the name of the extension), that should be an
>> +	 * error.  Names simply containing __ are dangerous to use, but should
>> +	 * be allowed.
>> +	 *
>> +	 * A future version of the GLSL specification will clarify this.
>>   	 */
>>   	if (strstr(identifier, "__")) {
>> -		glcpp_error (loc, parser, "Macro names containing \"__\" are reserved.\n");
>> +		glcpp_warning(loc, parser,
>> +			      "Macro names containing \"__\" are reserved "
>> +			      "for use by the implementation.\n");
>>   	}
>>   	if (strncmp(identifier, "GL_", 3) == 0) {
>>   		glcpp_error (loc, parser, "Macro names starting with \"GL_\" are reserved.\n");
>> diff --git a/src/glsl/glcpp/tests/086-reserved-macro-names.c.expected b/src/glsl/glcpp/tests/086-reserved-macro-names.c.expected
>> index d8aa9f0..5ca42a9 100644
>> --- a/src/glsl/glcpp/tests/086-reserved-macro-names.c.expected
>> +++ b/src/glsl/glcpp/tests/086-reserved-macro-names.c.expected
>> @@ -1,8 +1,8 @@
>> -0:1(10): preprocessor error: Macro names containing "__" are reserved.
>> +0:1(10): preprocessor warning: Macro names containing "__" are reserved for use by the implementation.
>>   
>>   0:2(9): preprocessor error: Macro names starting with "GL_" are reserved.
>>   
>> -0:3(9): preprocessor error: Macro names containing "__" are reserved.
>> +0:3(9): preprocessor warning: Macro names containing "__" are reserved for use by the implementation.
>>   
>>   
>>   
>>

All three patches worked as expecting with mesa-10.0.3 which I use right 
now.
Thank you.

Kind regards
Darius