Review: Needs Fixing

Hi Nicolae,

Looks good, thanks ;-) It's getting closer and closer to the git EBNFs!

Here are some comments:

1. Parser.y, line 2259: is "|| (block != NULL && block->isEmpty())" really 
needed? Will block not always be null if a BlockExpr is {} (see 
StatementsAndOptionalExpr nonterminal)?

2. Why is the OBJECT token treated specially and not mentioned in 
GeneralizedAtomicType like array/item/structured-item? Note that jn:object is 
no longer a function, as it was removed (I thought Markos removed it from 
Zorba, too). Then object should also be moved back to invalid function names.

Suggested test: object() throws a parsing error.

3. VersionDecl should not allow xquery in the JSONiq parser. Now, it seems to 
be allowed. A JSONiq query version declaration must be "jsoniq" (if a file 
begins with "xquery" the XQuery parser will be used anyway, so there is no way 
to test).

4. Why is append not authorized as a function? I thought it would be together 
with insert/rename/replace/delete.

Suggested test: local definition for local:append + default function namespace 
and query append().

5. "From" instead of "For" is implemented, but where is "select" instead of 
"return"? (this might require discussion about getting rid of "from", actually. 
I am not sure if the from/select decision was not reverted at some point.)

Suggested test: from $i in 1 to 10 select $i

6. Can you give me details and examples about the two new conflicts?

7. How is [[]] array handled? Is it parsed as a predicate with an array 
constructor and handled in the translator?

8. TRUE/FALSE/NULL/FROM(/SELECT?) should appear in the FUNCTION_NAME rule in a 
JSONIQ_PARSER #ifdef so that they can be used as function names. Or is this 
handled somewhere else?

Suggested test: false(), true(), null() -- from() with a local definition (see 
append above).

I hope it makes sense? Thanks!
Your team Zorba Coders is subscribed to branch lp:zorba.

Mailing list:
Post to     :
Unsubscribe :
More help   :

Reply via email to