bug-bison
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Bison 3.0: semantic predicates are mis-scanned as invalid directives


From: Akim Demaille
Subject: Re: Bison 3.0: semantic predicates are mis-scanned as invalid directives
Date: Wed, 16 Oct 2013 15:25:30 +0200

Le 13 oct. 2013 à 07:32, Rici Lake <address@hidden> a écrit :

Hi Rici,

Nice meeting you here!

> widget:
>       %?{  new_syntax } "widget" id new_args  { $$ = f($3, $4); }
> 
> Produces the error:
> 
> tst.y:5.8-10: error: invalid directive: ‘%?{’
>        %?{  new_syntax } "widget" id new_args  { $$ = f($3, $4); }
>        ^^^
> 
> (and others, resulting from not parsing new_syntax as a code block.)

This features does lack testing, that's unfortunate.

> Problem:
> 
> Line 269-271 of scan-gram.l:
> 
>  "%"{id}|"%"{notletter}([[:graph:]])+ {
>    complain (loc, complaint, _("invalid directive: %s"), quote (yytext));
>  }
> 
> definition of {notletter} at line 137 of the same file:
> 
> notletter [^.abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_]{-}[%\{]
> 
> (I don't understand why the '{' is backslash-escaped inside a character
> class,

Because it's clearer this way, including for syntax highlighters that
try to pair this { with the following ].

> nor why the {-} operator is used rather than just adding % and { to
> the list of "letters" in the first character class. But that's not the
> problem.)

Because {letter} is also used to define {id} (symbol names), and I
don't want "foo?" or "foo{" to be a symbol name.


> notletter will match a `?`, so the pattern at line 269 will match `%?{`.
> Since the pattern for semantic actions is later in the file (line 317), the
> "invalid directive" match takes precedence. Moving line 317 up won't help,
> though, because the pattern at 269 will match a longer string if there is
> no space after the %?{ in the source file.
> 
> Suggested fix:
> 
> Add ? to the list of "letters" in the definition of {notletter}. Consider
> cleaning up the definition a bit.
> 
> Possible workaround until fix is deployed:
> 
> Enter semantic predicates with a space after the `?`. This causes the
> pattern at line 269 to fail to match, but does not affect the pattern at
> line 317 because that pattern allows whitespace after the `?`

I chose something simpler: put the error handling clause where it
belongs: _after_ the proper clauses.  Well, technically I did the
converse.

I'll install the following patch in the maint branch.  Thanks again.

commit 4a25bde8c5876a5b013b0fbb96985d77f1edc79b
Author: Akim Demaille <address@hidden>
Date:   Wed Oct 16 15:19:44 2013 +0200

    glr: allow spaces between "%?" and "{" in predicates
    
    Reported by Rici Lake.
    http://lists.gnu.org/archive/html/bug-bison/2013-10/msg00004.html
    http://stackoverflow.com/questions/19330171/
    
    * src/scan-gram.l ("%?{"): Move before the error catching clause.
    * tests/glr-regression.at (Predicates): New test.

diff --git a/NEWS b/NEWS
index 7a2d4b5..e01198d 100644
--- a/NEWS
+++ b/NEWS
@@ -15,6 +15,11 @@ GNU Bison NEWS
   leaves "foo" diagnostics as warnings.  Similarly, with "-Werror=foo
   -Wno-error", "foo" diagnostics are now errors.
 
+*** GLR Predicates
+
+  As demonstrated in the documentation, one can now leave spaces between
+  "%?" and its "{".
+
 * Noteworthy changes in release 3.0 (2013-07-25) [stable]
 
 ** WARNING: Future backward-incompatibilities!
diff --git a/THANKS b/THANKS
index 2d4a6a9..0ffa8b6 100644
--- a/THANKS
+++ b/THANKS
@@ -110,6 +110,7 @@ R Blake                   address@hidden
 Raja R Harinath           address@hidden
 Ralf Wildenhues           address@hidden
 Richard Stallman          address@hidden
+Rici Lake                 address@hidden
 Rob Vermaas               address@hidden
 Robert Anisko             address@hidden
 Rob Conde                 address@hidden
diff --git a/src/scan-gram.l b/src/scan-gram.l
index 665e80d..6ec50ef 100644
--- a/src/scan-gram.l
+++ b/src/scan-gram.l
@@ -266,6 +266,13 @@ eqopt    ([[:space:]]*=)?
   "%pure"[-_]"parser"               DEPRECATED("%pure-parser");
   "%token"[-_]"table"               DEPRECATED("%token-table");
 
+  /* Semantic predicate. */
+  "%?"[ \f\n\t\v]*"{" {
+    nesting = 0;
+    code_start = loc->start;
+    BEGIN SC_PREDICATE;
+  }
+
   "%"{id}|"%"{notletter}([[:graph:]])+ {
     complain (loc, complaint, _("invalid directive: %s"), quote (yytext));
   }
@@ -313,13 +320,6 @@ eqopt    ([[:space:]]*=)?
     BEGIN SC_BRACED_CODE;
   }
 
-  /* Semantic predicate. */
-  "%?"[ \f\n\t\v]*"{" {
-    nesting = 0;
-    code_start = loc->start;
-    BEGIN SC_PREDICATE;
-  }
-
   /* A type. */
   "<*>"       return TAG_ANY;
   "<>"        return TAG_NONE;
diff --git a/tests/glr-regression.at b/tests/glr-regression.at
index 711ab7e..eea1c00 100644
--- a/tests/glr-regression.at
+++ b/tests/glr-regression.at
@@ -1749,3 +1749,30 @@ Cleanup: popping token 'a' ()
 ])
 
 AT_CLEANUP
+
+
+## ----------------------------------------------------------------- ##
+## Predicates.                                                       ##
+##                                                                   ##
+## http://lists.gnu.org/archive/html/bug-bison/2013-10/msg00004.html ##
+## ----------------------------------------------------------------- ##
+
+AT_SETUP([Predicates])
+
+# FIXME: We need genuine test cases with uses of %?.
+
+AT_DATA_GRAMMAR([input.y],
+[[%glr-parser
+%%
+// Exercise "%?{...}" and "%? {...}".
+widget:
+  %? { new_syntax } "widget" id new_args  { $$ = f($3, $4); }
+| %?{ !new_syntax } "widget" id old_args  { $$ = f($3, $4); }
+;
+
+%%
+]])
+
+AT_BISON_CHECK([[input.y]])
+
+AT_CLEANUP




reply via email to

[Prev in Thread] Current Thread [Next in Thread]