Re: [Mesa-dev] [PATCH] glcpp: error on multiple #else directives
On Fri, Oct 11, 2013 at 10:48 PM, Ian Romanick wrote:
> Carl,
>
> Can you look at this patch and Erik's follow-up patch? You (still) know
> the glcpp much better than any of the rest of us.
>
> (Carl is currently out of town, so I know his response will be slow...)
>
I guess "slow" doesn't mean *this* slow? :)
Should I resend the patch, with the #elseif fix squashed in?
> On 09/23/2013 01:35 PM, Erik Faye-Lund wrote:
>> The preprocessor currently eats multiple #else directives
>> int the same #if(def) ... #endif block. While I haven't been able
>> to find anything that explicitly disallows it, it's nonsensical
>> and should probably not be allowed.
>>
>> Add checks to reject the code.
>> ---
>>
>> I'm not entirely sure why parser->skip_stack can be NULL after
>> _glcpp_parser_skip_stack_change_if, but apparently it is for at
>> least one test.
>>
>> I'm also not entirely sure if this should be an error or a warning.
>> Thoughts?
>>
>> src/glsl/glcpp/glcpp-parse.y | 13 -
>> src/glsl/glcpp/glcpp.h| 1 +
>> src/glsl/glcpp/tests/118-multiple-else.c | 6 ++
>> src/glsl/glcpp/tests/118-multiple-else.c.expected | 8
>> 4 files changed, 27 insertions(+), 1 deletion(-)
>> create mode 100644 src/glsl/glcpp/tests/118-multiple-else.c
>> create mode 100644 src/glsl/glcpp/tests/118-multiple-else.c.expected
>>
>> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
>> index 6eaa5f9..885c64d 100644
>> --- a/src/glsl/glcpp/glcpp-parse.y
>> +++ b/src/glsl/glcpp/glcpp-parse.y
>> @@ -332,7 +332,17 @@ control_line:
>> }
>> }
>> |HASH_ELSE {
>> - _glcpp_parser_skip_stack_change_if (parser, & @1, "else", 1);
>> + if (parser->skip_stack &&
>> + parser->skip_stack->has_else)
>> + {
>> + glcpp_error(& @1, parser, "multiple #else");
>> + }
>> + else
>> + {
>> + _glcpp_parser_skip_stack_change_if (parser, & @1,
>> "else", 1);
>> + if (parser->skip_stack)
>> + parser->skip_stack->has_else = true;
>> + }
>> } NEWLINE
>> |HASH_ENDIF {
>> _glcpp_parser_skip_stack_pop (parser, & @1);
>> @@ -2012,6 +2022,7 @@ _glcpp_parser_skip_stack_push_if (glcpp_parser_t
>> *parser, YYLTYPE *loc,
>> node->type = SKIP_TO_ENDIF;
>> }
>>
>> + node->has_else = false;
>> node->next = parser->skip_stack;
>> parser->skip_stack = node;
>> }
>> diff --git a/src/glsl/glcpp/glcpp.h b/src/glsl/glcpp/glcpp.h
>> index 8aaa551..ccae96c 100644
>> --- a/src/glsl/glcpp/glcpp.h
>> +++ b/src/glsl/glcpp/glcpp.h
>> @@ -153,6 +153,7 @@ typedef enum skip_type {
>>
>> typedef struct skip_node {
>> skip_type_t type;
>> + bool has_else;
>> YYLTYPE loc; /* location of the initial #if/#elif/... */
>> struct skip_node *next;
>> } skip_node_t;
>> diff --git a/src/glsl/glcpp/tests/118-multiple-else.c
>> b/src/glsl/glcpp/tests/118-multiple-else.c
>> new file mode 100644
>> index 000..62ad49c
>> --- /dev/null
>> +++ b/src/glsl/glcpp/tests/118-multiple-else.c
>> @@ -0,0 +1,6 @@
>> +#if 0
>> +#else
>> +int foo;
>> +#else
>> +int bar;
>> +#endif
>> diff --git a/src/glsl/glcpp/tests/118-multiple-else.c.expected
>> b/src/glsl/glcpp/tests/118-multiple-else.c.expected
>> new file mode 100644
>> index 000..eaec481
>> --- /dev/null
>> +++ b/src/glsl/glcpp/tests/118-multiple-else.c.expected
>> @@ -0,0 +1,8 @@
>> +0:4(1): preprocessor error: multiple #else
>> +
>> +
>> +int foo;
>> +
>> +int bar;
>> +
>> +
>>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glcpp: error on multiple #else directives
Carl,
Can you look at this patch and Erik's follow-up patch? You (still) know
the glcpp much better than any of the rest of us.
(Carl is currently out of town, so I know his response will be slow...)
Thanks.
On 09/23/2013 01:35 PM, Erik Faye-Lund wrote:
> The preprocessor currently eats multiple #else directives
> int the same #if(def) ... #endif block. While I haven't been able
> to find anything that explicitly disallows it, it's nonsensical
> and should probably not be allowed.
>
> Add checks to reject the code.
> ---
>
> I'm not entirely sure why parser->skip_stack can be NULL after
> _glcpp_parser_skip_stack_change_if, but apparently it is for at
> least one test.
>
> I'm also not entirely sure if this should be an error or a warning.
> Thoughts?
>
> src/glsl/glcpp/glcpp-parse.y | 13 -
> src/glsl/glcpp/glcpp.h| 1 +
> src/glsl/glcpp/tests/118-multiple-else.c | 6 ++
> src/glsl/glcpp/tests/118-multiple-else.c.expected | 8
> 4 files changed, 27 insertions(+), 1 deletion(-)
> create mode 100644 src/glsl/glcpp/tests/118-multiple-else.c
> create mode 100644 src/glsl/glcpp/tests/118-multiple-else.c.expected
>
> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
> index 6eaa5f9..885c64d 100644
> --- a/src/glsl/glcpp/glcpp-parse.y
> +++ b/src/glsl/glcpp/glcpp-parse.y
> @@ -332,7 +332,17 @@ control_line:
> }
> }
> |HASH_ELSE {
> - _glcpp_parser_skip_stack_change_if (parser, & @1, "else", 1);
> + if (parser->skip_stack &&
> + parser->skip_stack->has_else)
> + {
> + glcpp_error(& @1, parser, "multiple #else");
> + }
> + else
> + {
> + _glcpp_parser_skip_stack_change_if (parser, & @1,
> "else", 1);
> + if (parser->skip_stack)
> + parser->skip_stack->has_else = true;
> + }
> } NEWLINE
> |HASH_ENDIF {
> _glcpp_parser_skip_stack_pop (parser, & @1);
> @@ -2012,6 +2022,7 @@ _glcpp_parser_skip_stack_push_if (glcpp_parser_t
> *parser, YYLTYPE *loc,
> node->type = SKIP_TO_ENDIF;
> }
>
> + node->has_else = false;
> node->next = parser->skip_stack;
> parser->skip_stack = node;
> }
> diff --git a/src/glsl/glcpp/glcpp.h b/src/glsl/glcpp/glcpp.h
> index 8aaa551..ccae96c 100644
> --- a/src/glsl/glcpp/glcpp.h
> +++ b/src/glsl/glcpp/glcpp.h
> @@ -153,6 +153,7 @@ typedef enum skip_type {
>
> typedef struct skip_node {
> skip_type_t type;
> + bool has_else;
> YYLTYPE loc; /* location of the initial #if/#elif/... */
> struct skip_node *next;
> } skip_node_t;
> diff --git a/src/glsl/glcpp/tests/118-multiple-else.c
> b/src/glsl/glcpp/tests/118-multiple-else.c
> new file mode 100644
> index 000..62ad49c
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/118-multiple-else.c
> @@ -0,0 +1,6 @@
> +#if 0
> +#else
> +int foo;
> +#else
> +int bar;
> +#endif
> diff --git a/src/glsl/glcpp/tests/118-multiple-else.c.expected
> b/src/glsl/glcpp/tests/118-multiple-else.c.expected
> new file mode 100644
> index 000..eaec481
> --- /dev/null
> +++ b/src/glsl/glcpp/tests/118-multiple-else.c.expected
> @@ -0,0 +1,8 @@
> +0:4(1): preprocessor error: multiple #else
> +
> +
> +int foo;
> +
> +int bar;
> +
> +
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glcpp: error on multiple #else directives
On Mon, Sep 23, 2013 at 11:02 PM, Erik Faye-Lund wrote: > On Mon, Sep 23, 2013 at 10:35 PM, Erik Faye-Lund wrote: >> The preprocessor currently eats multiple #else directives >> int the same #if(def) ... #endif block. While I haven't been able >> to find anything that explicitly disallows it, it's nonsensical >> and should probably not be allowed. >> >> Add checks to reject the code. >> --- >> >> I'm not entirely sure why parser->skip_stack can be NULL after >> _glcpp_parser_skip_stack_change_if, but apparently it is for at >> least one test. >> >> I'm also not entirely sure if this should be an error or a warning. >> Thoughts? >> Ping? Any thoughts? ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glcpp: error on multiple #else directives
On Mon, Sep 23, 2013 at 10:35 PM, Erik Faye-Lund wrote:
> The preprocessor currently eats multiple #else directives
> int the same #if(def) ... #endif block. While I haven't been able
> to find anything that explicitly disallows it, it's nonsensical
> and should probably not be allowed.
>
> Add checks to reject the code.
> ---
>
> I'm not entirely sure why parser->skip_stack can be NULL after
> _glcpp_parser_skip_stack_change_if, but apparently it is for at
> least one test.
>
> I'm also not entirely sure if this should be an error or a warning.
> Thoughts?
>
> src/glsl/glcpp/glcpp-parse.y | 13 -
> src/glsl/glcpp/glcpp.h| 1 +
> src/glsl/glcpp/tests/118-multiple-else.c | 6 ++
> src/glsl/glcpp/tests/118-multiple-else.c.expected | 8
> 4 files changed, 27 insertions(+), 1 deletion(-)
> create mode 100644 src/glsl/glcpp/tests/118-multiple-else.c
> create mode 100644 src/glsl/glcpp/tests/118-multiple-else.c.expected
>
> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
> index 6eaa5f9..885c64d 100644
> --- a/src/glsl/glcpp/glcpp-parse.y
> +++ b/src/glsl/glcpp/glcpp-parse.y
> @@ -332,7 +332,17 @@ control_line:
> }
> }
> | HASH_ELSE {
> - _glcpp_parser_skip_stack_change_if (parser, & @1, "else", 1);
> + if (parser->skip_stack &&
> + parser->skip_stack->has_else)
> + {
> + glcpp_error(& @1, parser, "multiple #else");
> + }
> + else
> + {
> + _glcpp_parser_skip_stack_change_if (parser, & @1,
> "else", 1);
> + if (parser->skip_stack)
> + parser->skip_stack->has_else = true;
> + }
Seems the HASH_ELIF variants needs the same treatment (sorry for the
b0rked white-space, gmail is not playing nice today):
diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
index 885c64d..690f2da 100644
--- a/src/glsl/glcpp/glcpp-parse.y
+++ b/src/glsl/glcpp/glcpp-parse.y
@@ -310,6 +310,11 @@ control_line:
_glcpp_parser_expand_and_lex_from (parser,
ELIF_EXPANDED, $2);
}
+ else if (parser->skip_stack &&
+ parser->skip_stack->has_else)
+ {
+ glcpp_error(& @1, parser, "#elif after #else");
+ }
else
{
_glcpp_parser_skip_stack_change_if (parser, & @1,
@@ -324,6 +329,11 @@ control_line:
{
glcpp_error(& @1, parser, "#elif with no expression");
}
+ else if (parser->skip_stack &&
+ parser->skip_stack->has_else)
+ {
+ glcpp_error(& @1, parser, "#elif after #else");
+ }
else
{
_glcpp_parser_skip_stack_change_if (parser, & @1,
diff --git a/src/glsl/glcpp/tests/119-elif-after-else.c
b/src/glsl/glcpp/tests/119-elif-after-else.c
new file mode 100644
index 000..9b9e923
--- /dev/null
+++ b/src/glsl/glcpp/tests/119-elif-after-else.c
@@ -0,0 +1,6 @@
+#if 0
+#else
+int foo;
+#elif 0
+int bar;
+#endif
diff --git a/src/glsl/glcpp/tests/119-elif-after-else.c.expected
b/src/glsl/glcpp/tests/119-elif-after-else.c.expected
new file mode 100644
index 000..33f0513
--- /dev/null
+++ b/src/glsl/glcpp/tests/119-elif-after-else.c.expected
@@ -0,0 +1,8 @@
+0:4(1): preprocessor error: #elif after #else
+
+
+int foo;
+
+int bar;
+
+
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
