Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Variable lookahead needed for ArrowTarget #1716

Open
GuntherRademacher opened this issue Jan 20, 2025 · 2 comments · May be fixed by #1763
Open

Variable lookahead needed for ArrowTarget #1716

GuntherRademacher opened this issue Jan 20, 2025 · 2 comments · May be fixed by #1763
Labels
Enhancement A change or improvement to an existing feature PR Pending A PR has been raised to resolve this issue XPath An issue related to XPath

Comments

@GuntherRademacher
Copy link
Contributor

The current grammar definition allows any QName (via EQName) as an ArrowStaticFunction:

ArrowTarget
         ::= ArrowStaticFunction ArgumentList
           | ArrowDynamicFunction PositionalArgumentList
ArrowStaticFunction
         ::= EQName
ArrowDynamicFunction
         ::= VarRef
           | InlineFunctionExpr
           | ParenthesizedExpr

This complicates the distinction of the static and dynamic variants of ArrowTarget, as it cannot be done with a fixed number of lookahead tokens. E.g. in an expression starting like this

A => fn ( $A, $B, $C, (: ... :) $Z ) { } (

the distinction cannot be made before the left brace is seen. While constructing an LR parser, there is a shift-reduce conflict between shifting fn as a keyword of an InlineFunctionExpr, or reducing fn to the QName of EQName.

This can easily be fixed by adding xgc: reserved-function-names to ArrowStaticFunction, which would also be consistent with other function calls in disallowing reserved function names:

ArrowStaticFunction
         ::= EQName
                          /* xgc: reserved-function-names */

But could not ArrowTarget also be written like the following?

ArrowTarget
         ::= FunctionCall
           | DynamicFunctionCall

In this case, the xgc: reserved-function-names constraint would be inherited from FunctionCall. It eliminates ArrowStaticFunction and ArrowDynamicFunction and at the same time lifts some restrictions imposed by the current ArrowTarget. It does not cause any LALR(2) conflicts.
@ChristianGruen
Copy link
Contributor

But could not ArrowTarget also be written like the following?

ArrowTarget
         ::= FunctionCall
           | DynamicFunctionCall

I would clearly be in favour of this simplification.

For example, it would allow us to write 1 => { 1: 'one', 2: 'two' }(), which is currently not possible. Currently, it is a bit arbitrary that some arrow targets need to be put into parentheses and others need not.

@ChristianGruen ChristianGruen added XPath An issue related to XPath Enhancement A change or improvement to an existing feature labels Jan 21, 2025
@michaelhkay
Copy link
Contributor

I think that the option to have an inline function on the RHS of => was a workaround that is no longer needed now we have the -> operator; we could happily revert from

ArrowDynamicFunction ::= VarRef |  InlineFunctionExpr |  ParenthesizedExpr

to what 3.1 allowed:

ArrowDynamicFunction ::= VarRef |  ParenthesizedExpr

However, the alternative of

ArrowTarget
         ::= FunctionCall
           | DynamicFunctionCall

certainly looks attractive. There's a technical objection in that the RHS isn't semantically a FunctionCall or DynamicFunctionCall, (which means that general statements we make about function calls might not apply to this kind of function call) but we can work around that.

I've been playing around a bit with operator precedence: consider

123 => map:entry(456)(123)

That seems to parse as 123 => (map:entry(456)(123)) not (as some users might expect) as (123 => map:entry(456)) (123), but that's OK, the rules seem to be clear -- and compatible with 3.1

@michaelhkay michaelhkay linked a pull request Feb 3, 2025 that will close this issue
Open
@michaelhkay michaelhkay added the PR Pending A PR has been raised to resolve this issue label Feb 3, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
Enhancement A change or improvement to an existing feature PR Pending A PR has been raised to resolve this issue XPath An issue related to XPath
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

1716 Generalize syntax of arrow expressions michaelhkay/qtspecs
3 participants
@ChristianGruen @michaelhkay @GuntherRademacher