[xslt] param and variable handling in EXSLT functions

[Shaun McCance <shaunm gnome org>]

I have a couple of problems with the handling of params and variables
inside of EXSLT functions.  This email will discuss the problems and
provide a fix for one of them.  The email is long.  Bear with me.

First, I understand that EXSLT functions aren't very well specified. 
The arguments I make shouldn't be construed as "my way is more correct",
but rather as "my way is better".  I think the behavior I argue of is
consistent with the spirit of the XSLT spec.  Two quotes:

http://www.exslt.org/func/elements/function/func.function.html
"The content of the func:function element is a template."

http://www.w3.org/TR/xslt#local-variables
"As well as being allowed at the top-level, both xsl:variable and
xsl:param are also allowed in templates. xsl:variable is allowed
anywhere within a template that an instruction is allowed. In this case,
the binding is visible for all following siblings and their descendants.
Note that the binding is not visible for the xsl:variable element
itself. xsl:param is allowed as a child at the beginning of an
xsl:template element. In this context, the binding is visible for all
following siblings and their descendants. Note that the binding is not
visible for the xsl:param element itself."

In my mind, a func:function should behave identically to an xsl:template
with respect to param and variable scoping.

Param Visibility
================
>From the function.8.xsl test file I've attached, here's a snippet that
shows the first problem I have with the current implementation:

<func:function name="func:testparam">
    <xsl:param name="foo" select="'Test'"/>
    <xsl:param name="bar" select="concat($foo, ' succeeded')"/>
    <func:result select="$bar"/>
</func:function>

If you call func:testparam with no arguments, both of the params will
get their default values from their select attributes.  In the current
imiplementation, however, evaluating $bar will fail, because $foo's
binding isn't visible to it.  This would not be the correct behavior
inside of an xsl:template, so I don't think we should regard it as good
behavior in a func:function.  My attached patch fixes this.

Variable Scoping
================
>From the function.9.xsl test file I've attached, here's a snippet that
shows the second problem I have:

<func:function name="func:testvar1">
    <xsl:variable name="foo" select="'foo'"/>
    <func:result select="func:testvar2()"/>
</func:function>
<func:function name="func:testvar2">
    <xsl:variable name="foo" select="'Test succeeded'"/>
    <func:result select="$foo"/>
</func:function>

Calling func:testvar1 results in an error, because $foo is redefined. 
This is because func:function doesn't constitue a new variable scope. 
This means that EXSLT functions that use variables can't call themselves
recursively.  It also means that a variable not define in a function may
be visible.

To me, the func:testvar2 doesn't qualify as "following siblings and
their descendants" of the xsl:variable declaration in func:testvar2. 
Line 1441 of libxslt/transform.c (inside xsltApplyOneTemplate) is the
start of the scoping stuff.  Basically, it makes a new scope iff the
stuff being processed is actually an xsl:template.

I would like func:function to constitute a new scope as well.  I don't
think I can fool xsltApplyOneTemplate into doing it for me.  However,
the set params (those actually passed to the function in the XPath) are
passed to xsltApplyOneTemplate in the params argument.  If I were to do
the scope setting stuff in exsltFuncFunctionFunction, it would end up
happening before the varsPush(ctxt, params).  Since there's a nice
comment there telling me "ordering of operations counts", that idea
doesn't sound very good to me.

Alternatively, I suppose I could do the varsPush myself and not pass the
set params to xsltApplyOneTemplate.  But I really prefer to pass as much
work as possible to xsltApplyOneTemplate.

Needless Computation
====================
My third problem is just an implementation problem.  It doesn't affect
the behavior at all.  Post-patch, the problem can be found on line 313
of libexslt/functions.c.  (Pre-patch, it's somewhere around there as
well.)  The offending line is

param = xsltParseStylesheetCallerParam(tctxt, paramNode);

This is for processing the set params.  What's happening here is that
the xsl:param is being processed and evaluated, and then the set value
is assigned onto the returned xsltStackElemPtr.  Consequently, when a
param value is actually passes, the default value is computed, even
though it's just thrown away.  That wastes CPU cycles.

Unfortunately, xsltParseStylesheetCallerParam is the only function in
libxslt that's public that turns an xsl:param into an xsltStackElemPtr. 
I'd really like some way of creating an xsltStackElemPtr that doesn't
compute the value.  I suppose I could xmlMalloc one myself, but that
doesn't seem like a very good thing to do.


OK, that's it.  Attached are seven files: libxslt-shaunm-030929-0.diff
is a fix for the first problem.  function.8.* are test files for the
first problem.  function.9.* are test files for the second problem,
which is not fixed by the patch.  Enjoy.

--
Shaun

Index: functions.c
===================================================================
RCS file: /cvs/gnome/libxslt/libexslt/functions.c,v
retrieving revision 1.17
diff -c -r1.17 functions.c
*** functions.c	18 Aug 2003 22:41:05 -0000	1.17
--- functions.c	29 Sep 2003 15:23:06 -0000
***************
*** 262,268 ****
      xmlXPathObjectPtr obj, oldResult, ret;
      exsltFuncData *data;
      exsltFuncFunctionData *func;
!     xmlNodePtr paramNode, oldInsert, fake;
      xsltStackElemPtr params = NULL, param;
      xsltTransformContextPtr tctxt = xsltXPathGetTransformContext(ctxt);
      int i;
--- 262,268 ----
      xmlXPathObjectPtr obj, oldResult, ret;
      exsltFuncData *data;
      exsltFuncFunctionData *func;
!     xmlNodePtr paramNode, oldInsert, fake, content;
      xsltStackElemPtr params = NULL, param;
      xsltTransformContextPtr tctxt = xsltXPathGetTransformContext(ctxt);
      int i;
***************
*** 289,296 ****
  	ctxt->error = XPATH_INVALID_ARITY;
  	return;
      }
!     if (func->content != NULL)
  	paramNode = func->content->prev;
      else
  	paramNode = NULL;
      if ((paramNode == NULL) && (func->nargs != 0)) {
--- 289,298 ----
  	ctxt->error = XPATH_INVALID_ARITY;
  	return;
      }
!     if (func->content != NULL) {
  	paramNode = func->content->prev;
+ 	content = func->content;
+     }
      else
  	paramNode = NULL;
      if ((paramNode == NULL) && (func->nargs != 0)) {
***************
*** 299,312 ****
  			 "param == NULL\n");
  	return;
      }
!     /* defaulted params */
      for (i = func->nargs; (i > nargs) && (paramNode != NULL); i--) {
- 	param = xsltParseStylesheetCallerParam (tctxt, paramNode);
- 	param->next = params;
- 	params = param;
  	paramNode = paramNode->prev;
      }
-     /* set params */
      while ((i-- > 0) && (paramNode != NULL)) {
  	obj = valuePop(ctxt);
  	/* FIXME: this is a bit hackish */
--- 301,312 ----
  			 "param == NULL\n");
  	return;
      }
! 
!     /* set params */
      for (i = func->nargs; (i > nargs) && (paramNode != NULL); i--) {
  	paramNode = paramNode->prev;
+ 	content = content->prev;
      }
      while ((i-- > 0) && (paramNode != NULL)) {
  	obj = valuePop(ctxt);
  	/* FIXME: this is a bit hackish */
***************
*** 328,334 ****
      oldInsert = tctxt->insert;
      tctxt->insert = fake;
      xsltApplyOneTemplate (tctxt, xmlXPathGetContextNode(ctxt),
! 			  func->content, NULL, params);
      tctxt->insert = oldInsert;
      if (params != NULL)
  	xsltFreeStackElemList(params);
--- 328,334 ----
      oldInsert = tctxt->insert;
      tctxt->insert = fake;
      xsltApplyOneTemplate (tctxt, xmlXPathGetContextNode(ctxt),
! 			  content, NULL, params);
      tctxt->insert = oldInsert;
      if (params != NULL)
  	xsltFreeStackElemList(params);
<?xml version="1.0"?>
<test>Test succeeded</test>
<test/>
<?xml version='1.0'?><!-- -*- Mode: xml -*- -->
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform";
                xmlns:func="http://exslt.org/functions";
                extension-element-prefixes="func"
                version="1.0">
<!--
This tests to make sure params are visible to following sibling
params inside an EXSLT function.
-->

<xsl:template match="/">
	<test>
		<xsl:value-of select="func:testparam()"/>
	</test>
</xsl:template>

<func:function name="func:testparam">
	<xsl:param name="foo" select="'Test'"/>
	<xsl:param name="bar" select="concat($foo, ' succeeded')"/>
	<func:result select="$bar"/>
</func:function>

</xsl:stylesheet>
<?xml version="1.0"?>
<test>Test succeeded</test>
<test/>
<?xml version='1.0'?><!-- -*- Mode: xml -*- -->
<xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform";
                xmlns:func="http://exslt.org/functions";
                extension-element-prefixes="func"
                version="1.0">
<!--
This tests to make sure variables defined in an EXSLT function aren't
visible in a function called from the function they're defined in.
-->

<xsl:template match="/">
	<test>
		<xsl:value-of select="func:testvar1()"/>
	</test>
</xsl:template>

<func:function name="func:testvar1">
	<xsl:variable name="foo" select="'foo'"/>
	<func:result select="func:testvar2()"/>
</func:function>

<func:function name="func:testvar2">
	<xsl:variable name="foo" select="'Test succeeded'"/>
	<func:result select="$foo"/>
</func:function>

</xsl:stylesheet>