fixed bug #171488 with cascading select in patterns. added test provided

* libxslt/pattern.c: fixed bug #171488 with cascading select in
  patterns.
* tests/general/bug-161.*, tests/general/Makefile.am,
  tests/docs/bug-161.*, tests/docs/Makefile.am: added test provided
  by Ben Ko
Daniel

ChangeLog

@@ -1,3 +1,11 @@
+Thu Mar 31 11:54:31 CEST 2005
+
+	* libxslt/pattern.c: fixed bug #171488 with cascading select in
+	  patterns.
+	* tests/general/bug-161.*, tests/general/Makefile.am,
+	  tests/docs/bug-161.*, tests/docs/Makefile.am: added test provided
+	  by Ben Ko
+	
 Thu Mar 31 00:28:38 CEST 2005 Daniel Veillard <daniel@veillard.com>
 
 	* tests/plugins/Makefile.am: fixed build outside of source tree

libxslt/pattern.c

@@ -99,6 +99,7 @@ struct _xsltCompMatch {
     const xmlChar *modeURI;      /* the mode URI */
     xsltTemplatePtr template;    /* the associated template */
 
+    int direct;
     /* TODO fix the statically allocated size steps[] */
     int nbStep;
     int maxStep;
@@ -147,6 +148,7 @@ xsltNewCompMatch(void) {
     cur->maxStep = 40;
     cur->nsNr = 0;
     cur->nsList = NULL;
+    cur->direct = 0;
     return(cur);
 }
 
@@ -367,6 +369,27 @@ xsltReverseCompMatch(xsltCompMatchPtr comp) {
 	i++;
     }
     comp->steps[comp->nbStep++].op = XSLT_OP_END;
+    /*
+     * detect consecutive XSLT_OP_PREDICATE indicating a direct
+     * matching should be done.
+     */
+    for (i = 0;i < comp->nbStep - 1;i++) {
+        if ((comp->steps[i].op == XSLT_OP_PREDICATE) &&
+	    (comp->steps[i + 1].op == XSLT_OP_PREDICATE)) {
+
+	    comp->direct = 1;
+	    if (comp->pattern[0] != '/') {
+		xmlChar *query;
+
+		query = xmlStrdup((const xmlChar *)"//");
+		query = xmlStrcat(query, comp->pattern);
+
+		xmlFree((xmlChar *) comp->pattern);
+		comp->pattern = query;
+	    }
+	    break;
+	}
+    }
 }
 
 /************************************************************************
@@ -400,6 +423,107 @@ xsltPatPushState(xsltStepStates *states, int step, xmlNodePtr node) {
     return(0);
 }
 
+/**
+ * xsltTestCompMatchDirect:
+ * @ctxt:  a XSLT process context
+ * @comp: the precompiled pattern
+ * @node: a node
+ *
+ * Test whether the node matches the pattern, do a direct evalutation
+ * and not a step by step evaluation.
+ *
+ * Returns 1 if it matches, 0 if it doesn't and -1 in case of failure
+ */
+static int
+xsltTestCompMatchDirect(xsltTransformContextPtr ctxt, xsltCompMatchPtr comp,
+	                xmlNodePtr node) {
+    xsltStepOpPtr sel = NULL;
+    xmlDocPtr prevdoc;
+    xmlDocPtr doc;
+    xmlXPathObjectPtr list;
+    int ix, j;
+    int nocache = 0;
+    int isRVT;
+
+    doc = node->doc;
+    if ((doc != NULL) &&
+	(doc->name != NULL) &&
+	(doc->name[0] == ' ') &&
+	(xmlStrEqual(BAD_CAST doc->name,
+		     BAD_CAST " fake node libxslt")))
+	isRVT = 1;
+    else
+	isRVT = 0;
+    sel = &comp->steps[0]; /* store extra in first step arbitrarily */
+
+    prevdoc = (xmlDocPtr)
+	XSLT_RUNTIME_EXTRA(ctxt, sel->previousExtra, ptr);
+    ix = XSLT_RUNTIME_EXTRA(ctxt, sel->indexExtra, ival);
+    list = (xmlXPathObjectPtr)
+	XSLT_RUNTIME_EXTRA_LST(ctxt, sel->lenExtra);
+    
+    if ((list == NULL) || (prevdoc != doc)) {
+	xmlXPathObjectPtr newlist;
+	xmlNodePtr parent = node->parent;
+	xmlDocPtr olddoc;
+	xmlNodePtr oldnode;
+
+	oldnode = ctxt->xpathCtxt->node;
+	olddoc = ctxt->xpathCtxt->doc;
+	ctxt->xpathCtxt->node = node;
+	ctxt->xpathCtxt->doc = doc;
+	newlist = xmlXPathEval(comp->pattern, ctxt->xpathCtxt);
+	ctxt->xpathCtxt->node = oldnode;
+	ctxt->xpathCtxt->doc = olddoc;
+	if (newlist == NULL)
+	    return(-1);
+	if (newlist->type != XPATH_NODESET) {
+	    xmlXPathFreeObject(newlist);
+	    return(-1);
+	}
+	ix = 0;
+
+	if ((parent == NULL) || (node->doc == NULL) || isRVT)
+	    nocache = 1;
+	
+	if (nocache == 0) {
+	    if (list != NULL)
+		xmlXPathFreeObject(list);
+	    list = newlist;
+
+	    XSLT_RUNTIME_EXTRA_LST(ctxt, sel->lenExtra) =
+		(void *) list;
+	    XSLT_RUNTIME_EXTRA(ctxt, sel->previousExtra, ptr) =
+		(void *) doc;
+	    XSLT_RUNTIME_EXTRA(ctxt, sel->indexExtra, ival) =
+		0;
+	    XSLT_RUNTIME_EXTRA_FREE(ctxt, sel->lenExtra) =
+		(xmlFreeFunc) xmlXPathFreeObject;
+	} else
+	    list = newlist;
+    }
+    if ((list->nodesetval == NULL) ||
+	(list->nodesetval->nodeNr <= 0)) {
+	if (nocache == 1)
+	    xmlXPathFreeObject(list);
+	return(0);
+    }
+    /* TODO: store the index and use it for the scan */
+    if (ix == 0) {
+	for (j = 0;j < list->nodesetval->nodeNr;j++) {
+	    if (list->nodesetval->nodeTab[j] == node) {
+		if (nocache == 1)
+		    xmlXPathFreeObject(list);
+		return(1);
+	    }
+	}
+    } else {
+    }
+    if (nocache == 1)
+	xmlXPathFreeObject(list);
+    return(0);
+}
+
 /**
  * xsltTestCompMatch:
  * @ctxt:  a XSLT process context
@@ -449,6 +573,7 @@ xsltTestCompMatch(xsltTransformContextPtr ctxt, xsltCompMatchPtr comp,
 	if (comp->modeURI != NULL)
 	    return(0);
     }
+
     i = 0;
 restart:
     for (;i < comp->nbStep;i++) {
@@ -667,6 +792,20 @@ restart:
 		int pos = 0, len = 0;
 		int isRVT;
 
+		/*
+		 * when there is cascading XSLT_OP_PREDICATE, then use a
+		 * direct computation approach. It's not done directly
+		 * at the beginning of the routine to filter out as much
+		 * as possible this costly computation.
+		 */
+		if (comp->direct) {
+		    if (states.states != NULL) {
+			/* Free the rollback states */
+			xmlFree(states.states);
+		    }
+		    return(xsltTestCompMatchDirect(ctxt, comp, node));
+		}
+
 		doc = node->doc;
 		if ((doc != NULL) &&
 		    (doc->name != NULL) &&
@@ -676,98 +815,10 @@ restart:
 		    isRVT = 1;
 		else
 		    isRVT = 0;
-		/*
-		 * The simple existing predicate code cannot handle
-		 * properly cascaded predicates. If in this situation
-		 * compute directly the full node list once and check
-		 * if the node is in the result list.
-		 */
-		if (comp->steps[i + 1].op == XSLT_OP_PREDICATE) {
-		    xmlDocPtr prevdoc;
-		    xmlXPathObjectPtr list;
-		    int ix, j;
-		    int nocache = 0;
 
-		    prevdoc = (xmlDocPtr)
-			XSLT_RUNTIME_EXTRA(ctxt, sel->previousExtra, ptr);
-		    ix = XSLT_RUNTIME_EXTRA(ctxt, sel->indexExtra, ival);
-		    list = (xmlXPathObjectPtr)
-			XSLT_RUNTIME_EXTRA_LST(ctxt, sel->lenExtra);
-		    
-		    if ((list == NULL) || (prevdoc != doc)) {
-			xmlChar *query;
-			xmlXPathObjectPtr newlist;
-			xmlNodePtr parent = node->parent;
-			xmlDocPtr olddoc;
-			xmlNodePtr oldnode;
-
-			if (comp->pattern[0] == '/')
-			    query = xmlStrdup(comp->pattern);
-			else {
-			    query = xmlStrdup((const xmlChar *)"//");
-			    query = xmlStrcat(query, comp->pattern);
-			}
-			oldnode = ctxt->xpathCtxt->node;
-			olddoc = ctxt->xpathCtxt->doc;
-			ctxt->xpathCtxt->node = node;
-			ctxt->xpathCtxt->doc = doc;
-			newlist = xmlXPathEval(query, ctxt->xpathCtxt);
-			ctxt->xpathCtxt->node = oldnode;
-			ctxt->xpathCtxt->doc = olddoc;
-			xmlFree(query);
-			if (newlist == NULL)
-			    return(-1);
-			if (newlist->type != XPATH_NODESET) {
-			    xmlXPathFreeObject(newlist);
-			    return(-1);
-			}
-			ix = 0;
-
-			if ((parent == NULL) || (node->doc == NULL) || isRVT)
-			    nocache = 1;
-			
-			if (nocache == 0) {
-			    if (list != NULL)
-				xmlXPathFreeObject(list);
-			    list = newlist;
-
-			    XSLT_RUNTIME_EXTRA_LST(ctxt, sel->lenExtra) =
-				(void *) list;
-			    XSLT_RUNTIME_EXTRA(ctxt, sel->previousExtra, ptr) =
-				(void *) doc;
-			    XSLT_RUNTIME_EXTRA(ctxt, sel->indexExtra, ival) =
-			        0;
-			    XSLT_RUNTIME_EXTRA_FREE(ctxt, sel->lenExtra) =
-				(xmlFreeFunc) xmlXPathFreeObject;
-			} else
-			    list = newlist;
-		    }
-		    if ((list->nodesetval == NULL) ||
-			(list->nodesetval->nodeNr <= 0)) {
-			if (nocache == 1)
-			    xmlXPathFreeObject(list);
-			goto rollback;
-		    }
-		    /* TODO: store the index and use it for the scan */
-		    if (ix == 0) {
-			for (j = 0;j < list->nodesetval->nodeNr;j++) {
-			    if (list->nodesetval->nodeTab[j] == node) {
-				if (nocache == 1)
-				    xmlXPathFreeObject(list);
-				goto found;
-			    }
-			}
-		    } else {
-		    }
-		    if (nocache == 1)
-			xmlXPathFreeObject(list);
-		    goto rollback;
-		}
 		/*
 		 * Depending on the last selection, one may need to
 		 * recompute contextSize and proximityPosition.
-		 *
-		 * TODO: make this thread safe !
 		 */
 		oldCS = ctxt->xpathCtxt->contextSize;
 		oldCP = ctxt->xpathCtxt->proximityPosition;

libxslt/xsltwin32config.h

@@ -44,7 +44,7 @@ extern "C" {
  *
  * extra version information, used to show a CVS compilation
  */
-#define LIBXSLT_VERSION_EXTRA "-CVS1006"
+#define LIBXSLT_VERSION_EXTRA "-CVS1008"
 
 /**
  * WITH_XSLT_DEBUG:

tests/docs/Makefile.am

@@ -160,6 +160,7 @@ EXTRA_DIST =	\
 	bug-158.xml bug-158.doc \
 	bug-159.xml \
 	bug-160.xml \
+	bug-161.xml \
 	character.xml \
 	array.xml \
 	items.xml

tests/docs/bug-161.xml

@@ -0,0 +1,5 @@
+<orderedlist>
+<listitem><para>test</para></listitem>
+<listitem><para>test</para></listitem>
+<listitem><para>test</para></listitem>
+</orderedlist>

tests/general/Makefile.am

@@ -169,6 +169,7 @@ EXTRA_DIST = \
     bug-158.out bug-158.xsl \
     bug-159.out bug-159.xsl \
     bug-160.out bug-160.xsl \
+    bug-161.out bug-161.xsl \
     character.out character.xsl \
     character2.out character2.xsl \
     itemschoose.out itemschoose.xsl \

tests/general/bug-161.out

@@ -0,0 +1,6 @@
+<?xml version="1.0"?>
+
+test
+test
+First element of last item of orderedlist<para>test</para>
+

tests/general/bug-161.xsl

@@ -0,0 +1,18 @@
+<xsl:stylesheet
+    xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
+    version="1.0">
+
+<xsl:output method="xml" /> 
+
+<!--match first element of last item of any orderedlist-->
+<!-- <xsl:template match="orderedlist/listitem[position()!=1][position()=last()]/*[1]"> -->
+<xsl:template match="orderedlist/listitem[position()!=1][position()=last()]/*[1]">
+   <xsl:text>First element of last item of orderedlist</xsl:text>
+   <xsl:copy>
+     <xsl:apply-templates select="@*|node()"/>
+   </xsl:copy>
+</xsl:template>
+
+</xsl:stylesheet>
+
+