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

NW for MK: Variadicity (166) #197

Merged
merged 16 commits into from
Nov 15, 2022
Merged

NW for MK: Variadicity (166) #197

merged 16 commits into from
Nov 15, 2022

Conversation

ndw
Copy link
Contributor

@ndw ndw commented Oct 10, 2022

This PR is intended to be technically equivalent to #166.

I've reworked a series of MK commits so that they are independent; I fear we would wind up with merge conflicts otherwise and cleaning up the merge conflicts, without accidentally making some other change, seemed riskier than just teasing them apart and making them independent before we accept and merge them.

@rhdunn rhdunn self-requested a review October 10, 2022 16:43
@Arithmeticus Arithmeticus self-requested a review October 13, 2022 02:40
<p>
<termdef id="dt-declared-function" term="declared function">The term
<term>declared function</term> is defined in <xspecref spec="XP40" ref="static_context"/>.
In refers to the definition of a function that can be called from within an XPath
Copy link
Contributor

@Arithmeticus Arithmeticus Oct 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In refers -> It refers. But the referent for "it" is unclear. Better: "It refers to a function...."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes to "It refers". But it (the term) doesn't refer to a "function" in the sense of "XDM function" or "function item". In 3.1 it was possible to gloss over the distinction, but this is no longer the case; the things in the static context ("declared function" objects) represent families of functions rather than single functions. (But there's a lot of legacy mess in this area and I haven't cleaned it up 100%. The 3.1 spec called this "function signatures" though it clearly contained more than the signature. And then it didn't really use it anyway, the binding rules for static function calls actually went to the dynamic context....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this very detailed review. As I work through it, I get to the point where it's hard to respond to every detailed point because changes that need to be made to one comment tend to invalidate subsequent comments. So I think the best approach is probably to attempt a second draft that takes into account as many of the comments as possible, focussing on those that are independent.

is used when function names are resolved statically, specifically when they
appear in static function calls or function references. The
<termdef id="dt-default-function-namespace" term="default function namespace">
<term>Default function namespace.</term> This is an algorithm that takes as input
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A namespace is not an algorithm.
Suggested: "This is the namespace URI returned by the algorithm that...."
Is the algorithm anywhere described or defined? Xref.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't discussed the capability of the default function namespace logic to support this additional complexity. As such, I would expect this to be the same text as the XQuery/XPath specs:

[Definition: Default function namespace. This is a namespace URI or absentDM31. The namespace URI, if present, is used for any unprefixed QName appearing in a position where a function name is expected.] The URI value is whitespace normalized according to the rules for the xs:anyURI type in Section 3.2.17 anyURI XS1-2 or Section 3.3.17 anyURI XS11-2.

</termdef>
The entries in this mapping define the set of
functions that are available to be called from a
<termdef id="dt-declared-functions"
Copy link
Contributor

@Arithmeticus Arithmeticus Oct 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there precedence in the specs for a definiendum being given definitions, one for the plural form and the other for the singular? Technically, the formula "Xs: the set of Xs" is circular, and should be rephrased in terms of the singular version, e.g., "Xs: the set of every X". But why is the plural version needed?
Further: "a set" -- i.e., arbitrary? Or is it every declared function declared from the static context?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this probably isn't ideal. Altogether, the terminology for the parts of the static and dynamic context is far from ideal, and it's difficult to sort out. Many of these "parts" are described using plural forms of the things they contain and I followed that convention, but it's not a good convention.

Copy link
Contributor

@Arithmeticus Arithmeticus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposal is excellent. I have flagged points that touch primarily on clarity, structure, and overlap. This is also my first attempt at reviewing changes to the specs, and I have focused primarily on questions of consistency. You may find a number of misfires on my part. Just tell my why and I'll try not to repeat the error in future reviews.

<termref def="dt-declared-function">declared functions</termref>.</termdef></p>

<p><termdef id="dt-declared-function" term="declared function">A <term>declared function</term>
contains the information needed to evaluate a static function call. It may be derived
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect the first sentence in a definition of a thing to be a predicate identifying the thing's superclass refined by its unique attributes, e.g., "A declared function is the set of all functions that are defined in the static context" or something like that. ("A human is a rational animal.")
"It may be derived": remote referent, assuming "it" = "declared function". If "call" is meant, perhaps rephrase.
"derived from": "be a member of"?

<p><termdef id="dt-declared-function" term="declared function">A <term>declared function</term>
contains the information needed to evaluate a static function call. It may be derived
from an actual declaration in a host language such as XQuery or XSLT, or it may represent
a built-in function, or it may represent information registered with the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"represent...represent...": "be...be..."?

The entries in this mapping define the set of
functions that are available to be called from a
<termdef id="dt-declared-functions"
term="declared functions">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I detect a structural problem in the document in this change. Here an attempt is made to define "declared function," absent the context of addressing functions in general (4.4). But there, the loss of a formal definition for "declared function" is felt in its juxtaposition with the definition for "dynamic function". There's some repetition, and overlap. My recommendation is to move the bulk of this material to 4.4 and supply an xref.

a built-in function, or it may represent information registered with the
processor using some <termref def="dt-implementation-defined"/> API.</termdef></p>

<p>The properties of a <termref def="dt-declared-function"/> include:</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If my suggestions for moving this material to 4.4 are adopted, I recommend using this material to define the property of both types of functions generally, before trying to say what properties are unique to static functions. Or point to the place in the specs where functions are defined in the abstract.

<item><p>The function name, which is an <termref def="dt-expanded-qname"/>.</p></item>
<item><p>A (possibly empty) list of required parameters, each having:</p>
<ulist>
<item><p>a parameter name (an <termref def="dt-expanded-qname"/>)</p></item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete "parameter"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete "parameter"

I think keeping parameter here makes sense as it is qualifying what the name is relating to.

with an arity range of 2 to 5. The named function reference <code>fn:format-date#3</code>
returns a (dynamic) function whose three arguments correspond to the first three arguments
of <code>fn:format-date</code>; the remaining two arguments will take their default values.
To obtain a 3-argument function that binds to arguments 1, 2, and 5 of <code>fn:format-date</code>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have expected this to be worded something like: "To deploy a 3-argument function call that binds..." As I understand it, what is obtained is the 5-argument function with arguments 3 and 4 bound to a default value or null, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, what is obtained is an arity-3 function item with the default values for the other two arguments present in its closure.

<g:ref name="TypeDeclaration"/>
</g:optional>
<g:optional name="OptionalDefaultForParam">
<g:string>:=</g:string>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In function calls, the = in := has been deleted (KeywordArgument). Will the discrepancy between : and := confuse users?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In function calls, the = in := has been deleted (KeywordArgument). Will the discrepancy between : and := confuse users?

This is using the $variable syntax to declare a variable (parameter in this case), so using := here is consistent with other variable declarations and bindings.

Using : in the function calls, that is not using the $variable syntax to assign the parameter (which would conflict with passing a variable of that name to the function). Instead, it is using a syntax akin to map construction, which uses the key : value syntax.

If/when function argument binding extends to map keys (such as for an option map), then the parameter and map key assignment is uniform and consistent with map construction.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We decided to use ":=".

for different kinds of parameter:</p>
relevant expression appears in the stylesheet, in the usual way. Note however that
for <elcode>xsl:param</elcode> elements defining <termref def="dt-function-parameter">function parameters</termref>,
the static context does not include variables bound in preceding-sibling <elcode>xsl:param</elcode> elements.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit?: preceding-sibling > sibling

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit?: preceding-sibling > sibling

This is correct. -- It is saying that if a previous xsl:param has a default expression that defines some variables, then those variables are not in the static context/scope of the current xsl:param.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Should we also call this out in the XPath and XQuery specs? -- Parameters declared before the current parameter's default expression are not included in the static context of that expression.


<p>Unlike other contexts where <elcode>xsl:param</elcode> elements may appear, the static context for
the select expression and contained sequence constructor of an <elcode>xsl:param</elcode> child of <elcode>xsl:function</elcode>
does not include variable bindings appearing it its preceding-sibling <elcode>xsl:param</elcode> elements. This means
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it its > in its

<termref def="dt-import-precedence"/> than <var>G</var>, and the
<termref def="dt-arity-range"/>
of <var>G</var> includes part but not all of the arity range of <var>F</var>,
unless <var>G</var> is itself <termref def="dt-eclipsed"/> by another
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revise?: "by another xsl:function declaration that also successfully eclipses F."

<g:optional name="OptionalTypeDeclarationForParamWithDefault">
<g:ref name="TypeDeclaration"/>
</g:optional>
<g:optional name="OptionalDefaultForParam">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency with the above, this should be named OptionalDefaultForParamWithDefault.

<g:ref name="FamilyParamList"/>
</g:optional>
<g:string>)</g:string>
<g:optional name="optionFuncFamilyType">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should start with a capital 'O' for consistency with the other optional names.

<g:ref name="TypeDeclaration"/>
</g:optional>
<g:optional name="OptionalDefaultForParam">
<g:string>:=</g:string>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In function calls, the = in := has been deleted (KeywordArgument). Will the discrepancy between : and := confuse users?

This is using the $variable syntax to declare a variable (parameter in this case), so using := here is consistent with other variable declarations and bindings.

Using : in the function calls, that is not using the $variable syntax to assign the parameter (which would conflict with passing a variable of that name to the function). Instead, it is using a syntax akin to map construction, which uses the key : value syntax.

If/when function argument binding extends to map keys (such as for an option map), then the parameter and map key assignment is uniform and consistent with map construction.

@@ -1860,9 +1891,9 @@ ErrorVal ::= "$" VarName
</g:production>

<g:production name="KeywordArgument" if="xpath40 xquery40 xslt40-patterns">
<g:ref name="NCName"/>
<g:string>:=</g:string>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment on the parameters. -- This is consistent with the way map construction works.

To me, := is for variable assignment which would require a $name variable syntax. That would be ambiguous with passing a variable of that name to the function. It would also be less clear as to what is happening, making the code harder to read. Using := with $name (if we used $name instead of just name) would break the Scripting Extension assignment expressions which allow you to do things like $name := 123 to update the value of a variable.

is used when function names are resolved statically, specifically when they
appear in static function calls or function references. The
<termdef id="dt-default-function-namespace" term="default function namespace">
<term>Default function namespace.</term> This is an algorithm that takes as input
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't discussed the capability of the default function namespace logic to support this additional complexity. As such, I would expect this to be the same text as the XQuery/XPath specs:

[Definition: Default function namespace. This is a namespace URI or absentDM31. The namespace URI, if present, is used for any unprefixed QName appearing in a position where a function name is expected.] The URI value is whitespace normalized according to the rules for the xs:anyURI type in Section 3.2.17 anyURI XS1-2 or Section 3.3.17 anyURI XS11-2.

contains the information needed to evaluate a static function call. It may be derived
from an actual declaration in a host language such as XQuery or XSLT, or it may represent
a built-in function, or it may represent information registered with the
processor using some <termref def="dt-implementation-defined"/> API.</termdef></p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or it may represent information registered with the processor using some API.

I find this part slightly confusing, as in order to define a function via an API you are not just registering information, you are defining the function. Maybe change this to:

or it may be registered with the processor using some API.

<item><p>The function name, which is an <termref def="dt-expanded-qname"/>.</p></item>
<item><p>A (possibly empty) list of required parameters, each having:</p>
<ulist>
<item><p>a parameter name (an <termref def="dt-expanded-qname"/>)</p></item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete "parameter"

I think keeping parameter here makes sense as it is qualifying what the name is relating to.

for different kinds of parameter:</p>
relevant expression appears in the stylesheet, in the usual way. Note however that
for <elcode>xsl:param</elcode> elements defining <termref def="dt-function-parameter">function parameters</termref>,
the static context does not include variables bound in preceding-sibling <elcode>xsl:param</elcode> elements.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit?: preceding-sibling > sibling

This is correct. -- It is saying that if a previous xsl:param has a default expression that defines some variables, then those variables are not in the static context/scope of the current xsl:param.

@cmsmcq
Copy link

cmsmcq commented Oct 18, 2022

Three minor queries about declared functions:

  • In 2.1.1, the replacement of the definition of statically known function signatures with definitions of declared function and declared functions has as a consequence that while the status quo explicitly says that those function signatures are available for reference from a named function reference, the new text does not. Is that covered elsewhere? If not, is it likely to matter?

  • Working through the proposal I find myself increasingly uncomfortable that the denotation of declared function is not a function. I wonder if the term function declaration would work better? Or function family?

  • I note that the description of the default function namespace allows implementation-defined algorithms for searching multiple namespaces and resolving conflicts; this makes me wonder whether we have a compelling reason to forbid such algorithms for resolving overlapping arity ranges. (I don't think I have a preference, beyond adhering to the goose/gander principle.)

@michaelhkay
Copy link
Contributor

@cmsmcq

In 2.1.1, the replacement of the definition of statically known function signatures with definitions of declared function and declared functions has as a consequence that while the status quo explicitly says that those function signatures are available for reference from a named function reference, the new text does not. Is that covered elsewhere? If not, is it likely to matter?

I think we used to say it twice, we now only say it once. It's there in 4.4.2.3.

Working through the proposal I find myself increasingly uncomfortable that the denotation of declared function is not a function. I wonder if the term function declaration would work better? Or function family?

Yes, me too. I was trying to minimise change to the 3.1 spec, but I'm increasingly persuaded that the 3.1 presentation which tries to maintain that there are two kinds of function, static functions and dynamic functions, is wrong. I'm now proposing to call the things in the static context "function definitions". I don't want to call them "function declarations" because that's a specific piece of syntax in XQuery, but I think that the name "function definitions" gets us away from the idea that a declared function is a kind of function.

I note that the description of the default function namespace allows implementation-defined algorithms for searching multiple namespaces and resolving conflicts; this makes me wonder whether we have a compelling reason to forbid such algorithms for resolving overlapping arity ranges. (I don't think I have a preference, beyond adhering to the goose/gander principle.)

Unfortunately, the base text I was starting from included a speculative enhancement for resolving unprefixed function names, which is unrelated to this proposal. I'm trying to roll that back, but the changes muddy the diff.

@ndw
Copy link
Contributor Author

ndw commented Oct 19, 2022

Formatted versions of the specifications with this PR applied are available

@cmsmcq
Copy link

cmsmcq commented Oct 25, 2022

Some incomplete and unsystematic notes:

  • Is it intended that in section 2.2.1 Static Context of XPath, in the description of function definition, both required type and default value are glossed as "(a sequence type)"? It seems unexpected enough that I suspect a copy/paste error.
  • I think the sentence "It is not acceptable to have a function definition with one required parameter and another (with the same name) having three optional parameters", would do its work better without the parenthesis marks.
  • A propos the sentence "Implementations must ensure that no two declared functions have the same expanded QName and overlapping arity ranges (even if the signatures are consistent)."
    • The term "declared function" seems important and is used in several places, but is nowhere defined in the XPath spec. Does it mean a function definition in the statically known function definitions? ... in the dynamically known function definitions?
    • The requirement that no two declared functions have certain properties entails that we need a clean story about how to tell whether a declared function f1 is the same declared function as a declared function f2, or they are different functions. (I suspect this need is particularly acute when there is some possibility that the same module definition [for some definition of "same"] might by accident or design be referenced twice under different URIs, but the need is probably more general. I don't think we want complicated rules, just a clear one.)
  • In section 2.3.4 Consistency constraints, the last bullet item appears to be redundant. It says "Every function definition in the statically-known function definitions of the static context must also be present in the dynamically known function definitions of the dynamic context", which follows from the statement in the definition of the dynamically known function definitions property that "It includes the statically-known function definitions as a subset, but may include other function definitions that are not known statically." For economy of expression, one of these should probably be deleted; alternatively, one should be marked as a redundant reminder of the normative statement made elsewhere. (I would make the definition be the normative statement and drop the bullet item or label it a consequence of the definition.)
  • In 4.4.1 Function definitions, the sentence "The function is evaluated by executing its implementation, ..." makes my blood run cold. I do not now have a proposal for alternative wording, but the sudden irruption of an imperative, procedural description into what is otherwise a declarative specification seems out of place. Perhaps the change could just be to "The function is evaluated using a dynamic context that provides values for all the declared parameters, initialized as described in 4.4.1.2 Evaluating Static Function Calls below."
  • In section 4.4.1.1 Static Function Call Syntax, missing text (bad hyperlink?) in the sentence "If the EQName in a static function call is an unprefixed lexical QName, it is expanded using the default function namespace in the ." In the next paragraph, missing whitespace in "in the static contextusing the rules". (Other instances of missing whitespace immediately following a hyperlink are also visible, but I won't try to list them all.)
  • Section 4.4.1.2 Evaluating Static Function Calls says, inter alia: "Any parameters that have not been associated with any argument expression by applying the above rules (which are necessarily optional parameters) are then associated with the default expression for the relevant parameter in the declared function."
    It is not clear to me that the rules given earlier in the document guarantee that parameters not yet associated with any argument expression will be optional parameters. (I found this sentence while looking for the place where the text would say that it is an error if at this point any required parameter does not yet have an argument expression.)
    In particular, consider a function definition named my:foo with two required and two optional parameters named A, B, C, and D. I think the description of function definitions in section 2.2.1 Static Context tells us that its arity range is two to four, inclusive. Consider then a static function call my:foo(23, C := 42, D := "skiddoo"), with one position and two keyword parameters. I think item 1 in section 4.4.1.2 tells us the required arity is three. That falls into the range two to four inclusive, so the call to my:foo is bound to the function definition for my:foo. I think item 2 in the same section then tells us that A is bound to the expression 23, C to 42, and D to "skiddoo".
    Item 2 then tells me that any parameter that doesn't have an expression (here B) is "necessarily" an optional parameter. But B is not an optional parameter. It is false to say that it is, and it is doubly false to say that it is necessarily an optional parameter.
    Perhaps I need to say that I take "necessarily" in this sentence to mean that the claim follows as a logical consequence of the rules already given. If "necessarily" is meant here to mean that the state of affairs described is a pre-condition of error-free evaluation, then I think we would do better to find another way of making the point.
  • In item 3 of the same list, some unmarked legacy text says "it is not required that an argument be evaluated if the function can evaluate its body without evaluating that argument." I think this would be clearer if rephrased as "it is not required that an argument be evaluated if the function body can be evaluated without evaluating that argument." Or "it is not required that an argument be evaluated if the function body can be evaluated it."

Copy link
Contributor

@Arithmeticus Arithmeticus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The revision is much clearer, and the simpler approach is to be preferred. Comments address only stylistic issues.


<p>The names of the parameters must be distinct.</p>

<p><termdef id="dt-arity-range" term="arity range">A <termref def="dt-function-definition"/> has an <term>arity range</term>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> , (in this context "which" is non-restrictive)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in my years of doing this job I've learned that Americans are much fussier about which/that distinctions than Brits.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it's just the comma, not the which/that distinction, which the Cambridge Grammar of English Language has convinced me of as being unsound, pace Garner.

the functions specified in <bibref ref="xpath-functions-40"/>.</termdef></p>


<p>Implementations must ensure that no two declared functions have the same <termref
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

functions -> function definitions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes.

about a family of functions with the same name and a defined arity range. These functions
are in most cases known statically (they appear in the <termref def="dt-statically-known-function-definitions"/>),
but there may be further function definitions
that are only known dynamically (appearing in the <termref def="dt-dynamically-known-function-definitions"/>).</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only known -> known only


<termdef id="dt-statically-known-function-definitions"
term="statically-known function definitions">
<term>Statically-known function definitions.</term> This is a set of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Statically-known -> Statically known
The non-hyphenated form predominates 5:1, and is (in my editorial circles) to be preferred in constructions of [-ly adverb] [adj.] [noun].

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, again I've gradually learned that hyphenating adverb-adjective phrases seems to be much more the norm in BrE than in AmE. (The Oxford Manual of Style says use no hyphen if the adverb ends in "-ly", but use one otherwise (so "a well-known and happily married couple". Bizarre.)

@michaelhkay
Copy link
Contributor

michaelhkay commented Oct 25, 2022 via email

Copy link
Contributor

@rhdunn rhdunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this looks good. I've left some comments, and have comments from my previous review.

<p>The names of the parameters must be distinct.</p>

<p><termdef id="dt-arity-range" term="arity range">A <termref def="dt-function-definition"/> has an <term>arity range</term>
which is a range of consecutive non-negative integers. If the function definition has <var>M</var> required parameters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the use of range in "is a range of consecutive" is circular here. I think it would be better to use something like "list" or "sequence" instead.

argument is then associated with the corresponding parameter.</p></item>
<item><p>Any parameters that have not been associated with any argument expression
by applying the above rules (which are necessarily optional parameters) are then associated
with the default expression for the relevant parameter in the declared function.</p></item>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need rules to handle the cases where:

  1. a named keyword does not match a parameter name in the function -- This will likely be made into the case of adding it into a map parameter, where it would be relevant in the case where no suitable map parameter exists.
  2. any positional parameters have not been associated with either positional arguments nor named arguments

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now resolved.

Each argument expression established by the above rules is evaluated with respect to DC.
The order of argument evaluation is <termref def="dt-implementation-dependent"/> and it is not
required that an argument be evaluated if the function can evaluate its body without evaluating
that argument.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I'm unsure how this works (vis-a-vis the implementation defined evaluation order) for default parameter expressions that reference any preceding parameter names (like how let variable binding expressions can reference preceding variable bindings -- e.g. let $x := 1, $y := $x + 1).

It may be better to add the following at the end:

The argument evaluation must occur at least at the point at which the parameter name it is bound to is first used in the function, either within the body of the function or from the evaluation of one of the default parameter expressions.

  1. Does this also apply to default parameter expressions? Can they also be lazily evaluated?

  2. I can see potential incompatibilities where evaluating a parameter will raise an error, but some calls don't make use of that parameter. Because evaluation is implementation defined, some implementations will raise an error and others won't.

<g:choice name="FunctionDeclBody">
<g:ref name="FunctionBody" if="xquery40"/>
<g:ref name="External"/>
</g:choice>
</g:production>

<g:production name="FunctionSignatureWithDefaults">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way this is defined means that functions can be defined with an interleaved mix of optional and required parameters (because ParamListWithDefaults is a sequence of one or more ParamWithDefault types that are parameters with optional default expressions.

Is this intentional? If so, I'm not sure how it would work. -- It would effectively make any non-default parameter after the first requiring the keyword argument calling convention and not supporting positional arguments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: This also breaks things like the specification of arity ranges in err:XQST0034, etc. that reference required parameters and optional parameters.

@@ -1145,7 +1145,7 @@ of the declared type).</p></item><item role="xquery"><p>Limits on ranges of valu

<bibl id="xpath-datamodel-31" key="XQuery and XPath Data Model (XDM) 3.1"/>

<bibl id="xpath-functions-31" key="XQuery and XPath Functions and Operators 3.1"/>
<bibl id="xpath-functions-40" key="XQuery and XPath Functions and Operators 4.0"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing:

ERROR: NO xpath-functions-40 KNOWN!

here.

@ndw ndw mentioned this pull request Nov 11, 2022
@ndw
Copy link
Contributor Author

ndw commented Nov 15, 2022

Approved at meeting 011, 15 November 2022

@ndw ndw merged commit dde277f into qt4cg:master Nov 15, 2022
@ndw ndw deleted the nw-166-variadicity branch November 15, 2022 17:29
ndw added a commit to ndw/qtspecs-xslt4 that referenced this pull request Nov 16, 2022
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

5 participants