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

diff --git a/ChangeLog b/ChangeLog
index bb5bd82..0881392 100644 (file)
--- a/ChangeLog
+++ b/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

diff --git a/libxslt/pattern.c b/libxslt/pattern.c
index 9fea5a7..015d944 100644 (file)
--- a/libxslt/pattern.c
+++ b/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;
+       }
+    }
 }
 
 /************************************************************************
@@ -401,6 +424,107 @@ xsltPatPushState(xsltStepStates *states, int step, xmlNodePtr node) {
 }
 
 /**
+ * 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
  * @comp: the precompiled pattern
@@ -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;

diff --git a/libxslt/xsltwin32config.h b/libxslt/xsltwin32config.h
index 4e228a1..c0a0a1e 100644 (file)
--- a/libxslt/xsltwin32config.h
+++ b/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:

diff --git a/tests/docs/Makefile.am b/tests/docs/Makefile.am
index e7da722..89aff9a 100644 (file)
--- a/tests/docs/Makefile.am
+++ b/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

diff --git a/tests/docs/bug-161.xml b/tests/docs/bug-161.xml
new file mode 100644 (file)
index 0000000..b1548f3
--- /dev/null
+++ b/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>

diff --git a/tests/general/Makefile.am b/tests/general/Makefile.am
index 733f9cf..0f5a89b 100644 (file)
--- a/tests/general/Makefile.am
+++ b/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 \

diff --git a/tests/general/bug-161.out b/tests/general/bug-161.out
new file mode 100644 (file)
index 0000000..20b1516
--- /dev/null
+++ b/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>
+

diff --git a/tests/general/bug-161.xsl b/tests/general/bug-161.xsl
new file mode 100644 (file)
index 0000000..91bfe7f
--- /dev/null
+++ b/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>
+
+