Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add != (not equal) to list of supported binops #34

Merged
merged 6 commits into from Oct 24, 2012
Merged

Add != (not equal) to list of supported binops #34

merged 6 commits into from Oct 24, 2012

Conversation

dgryski
Copy link
Contributor

@dgryski dgryski commented Oct 23, 2012

I was munging some data and needed to query "select (.some.json.field != 200)".

Not all versions of flex export the declaration of yyscan_t, so I had to add it by hand. Ugly, but "error: unknown type name 'yyscan_t'" is a common error message on Google.

Also committing the new output of lex/yacc.

@stedolan
Copy link
Contributor

This looks good, but the yyscan_t thing is a bit fugly.

The issue seems to be that we need lexer.gen.h to be included quite early in the parser, but lexer.gen.h requires that various things are already defined by the parser (YYSTYPE in particular). Those are also defined by parser.gen.h, so I think if we include parser.gen.h and then lexer.gen.h at the top of the parser it should work.

This change to parser.y works fine on my flex/bison, could you test it on yours?

diff --git a/parser.y b/parser.y
index 0effe07..f4ce96c 100644
--- a/parser.y
+++ b/parser.y
@@ -2,6 +2,9 @@
 #include <stdio.h>
 #include <string.h>
 #include "compile.h"
+#include "parser.gen.h"
+#include "lexer.gen.h"
+
 %}
 %code requires {
 #include "locfile.h"
@@ -83,7 +86,6 @@

 %type <blk> Exp Term MkDict MkDictPair ExpD ElseBody QQString FuncDef FuncDefs 
 %{
-#include "lexer.gen.h"
 #define FAIL(loc, msg)                                   \
   do {                                                   \
     location l = loc;                                    \

If that works, we can ditch the yyscan_t hack.

@dgryski
Copy link
Contributor Author

dgryski commented Oct 23, 2012

I did a bit more investigating. It appears that the problem comes from the fact that the latest bison (2.6.2, possibly earlier) generates a prototype for yyparse() and puts it in parser.gen.h. Since we're using %parse-param to add a yyscan_t param, we've just created a circular dependency in the #includes. lexer.gen.h needs the token definitions from parser.gen.h, and parser.gen.h needs the yyscan_t definition from lexer.gen.h .

I don't see a nice way out of this :(

@stedolan
Copy link
Contributor

It's not like combining flex and bison is an unusual move, you'd think this stuff would be simpler.

I think we can wrap the lexer parameter in a forward-declared struct, so %parse-param sees an opaque pointer to a struct we don't define until we #include lexer.gen.h.

Try this (again, works on my flex/bison): https://gist.github.com/3940705

@stedolan
Copy link
Contributor

Also: docs and tests! You get away without writing docs this time, since apparently I've documented != without actually implementing it (?!).

Tests are pretty easy: there's a file called testdata that contains a bunch of tests which consist of program, sample input and expected output. When you run make test, the test cases are run under valgrind and make gets angry at you if they either failed or leaked.

@dgryski
Copy link
Contributor Author

dgryski commented Oct 23, 2012

The patch almost works, but I get "warning: 'struct lexer_param' declared inside parameter list " while trying to compile parser.gen.h from within lexer.l . (Because at this point the compiler hasn't seen any forward declaration for the struct). Adding an additional 'struct lexer_param;' to the top of lexer.l fixes that too.

I'll add some tests for != .

@stedolan
Copy link
Contributor

Cool. Adding another forward declaration struct lexer_param in lexer just before the #include should fix that, I think.

This prevents the circuluar dependency between parser.gen.h and
lexer.gen.h.  Newer versions of bison add a prototype for yyparse() to
parser.gen.h that include the as-yet-undeclared yyscan_t type.
@dgryski
Copy link
Contributor Author

dgryski commented Oct 24, 2012

I've added some tests cases and also pulled in your fix for the circular dependency problem.

stedolan added a commit that referenced this pull request Oct 24, 2012
Add != (not equal) to list of supported binops
@stedolan stedolan merged commit 033d9b2 into jqlang:master Oct 24, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants