Merge lp:~zorba-coders/zorba/no-copy into lp:zorba

[Markos Zaharioudakis]

1. Put back markForSerialization() (renamed as markInUsafeContext())
2. path expr is unsafe if it contains any KindTest with type check and the construction mode i strip
3. copying is unsafe only if ns_inherit and ns_preserve

1. Put back markForSerialization() (renamed as markInUsafeContext())
2. path expr is unsafe if it contains any KindTest with type check and the construction mode i strip
3. copying is unsafe only if ns_inherit and ns_preserve

[lp:~zorba-coders/zorba/no-copy]

11077. By Markos Zaharioudakis on 2012-10-02

put back markForSerialization() + path expr is unsafe if it contains any KindTest with type and the construction mode i strip + copying is unsafe only if ns_inherit and ns_preserve

[Markos Zaharioudakis (markos-za)]

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/no-copy-2012-10-02T22-59-53.733Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job no-copy-2012-10-02T22-59-53.733Z is finished. The final status was:

All tests succeeded!

=== modified file 'src/compiler/expression/expr_base.cpp'
--- src/compiler/expression/expr_base.cpp	2012-10-02 11:57:15 +0000
+++ src/compiler/expression/expr_base.cpp	2012-10-02 22:52:19 +0000
@@ -541,6 +541,30 @@
 /*******************************************************************************
 
 ********************************************************************************/
+BoolAnnotationValue expr::getInUnsafeContext() const
+{
+  return (BoolAnnotationValue)
+         ((theFlags1 & IN_UNSAFE_CONTEXT_MASK) >> IN_UNSAFE_CONTEXT);
+}
+
+
+void expr::setInUnsafeContext(BoolAnnotationValue v)
+{
+  theFlags1 &= ~IN_UNSAFE_CONTEXT_MASK;
+  theFlags1 |= (v << IN_UNSAFE_CONTEXT);
+}
+
+
+bool expr::inUnsafeContext() const
+{
+  BoolAnnotationValue v = getInUnsafeContext();
+  return (v == ANNOTATION_TRUE || v == ANNOTATION_TRUE_FIXED);
+}
+
+
+/*******************************************************************************
+
+********************************************************************************/
 BoolAnnotationValue expr::getContainsPragma() const
 {
   return (BoolAnnotationValue)
 
=== modified file 'src/compiler/expression/expr_base.h'
--- src/compiler/expression/expr_base.h	2012-10-02 11:57:15 +0000
+++ src/compiler/expression/expr_base.h	2012-10-02 22:52:19 +0000
@@ -147,8 +147,9 @@
     UNFOLDABLE              = 10,
     CONTAINS_RECURSIVE_CALL = 12,
     PROPAGATES_INPUT_NODES  = 14,
-    MUST_COPY_NODES         = 16,
-    CONTAINS_PRAGMA         = 18
+    IN_UNSAFE_CONTEXT       = 16,
+    MUST_COPY_NODES         = 18,
+    CONTAINS_PRAGMA         = 20
   } Annotationkey;
 
   typedef enum
@@ -161,8 +162,9 @@
     UNFOLDABLE_MASK               = 0xC00,
     CONTAINS_RECURSIVE_CALL_MASK  = 0x3000,
     PROPAGATES_INPUT_NODES_MASK   = 0xC000,
-    MUST_COPY_NODES_MASK          = 0x30000,
-    CONTAINS_PRAGMA_MASK          = 0xC0000
+    IN_UNSAFE_CONTEXT_MASK        = 0x30000,
+    MUST_COPY_NODES_MASK          = 0xC0000,
+    CONTAINS_PRAGMA_MASK          = 0x300000
   } AnnotationMask;
 
 
@@ -313,6 +315,13 @@
 
   void setMustCopyNodes(BoolAnnotationValue v);
 
+  // Annotation : inUnsafeContext
+  BoolAnnotationValue getInUnsafeContext() const;
+
+  void setInUnsafeContext(BoolAnnotationValue v);
+
+  bool inUnsafeContext() const;
+
   // Annotation : containsPragma
   BoolAnnotationValue getContainsPragma() const;
 
 
=== modified file 'src/compiler/rewriter/rules/nodeid_rules.cpp'
--- src/compiler/rewriter/rules/nodeid_rules.cpp	2012-10-02 11:57:15 +0000
+++ src/compiler/rewriter/rules/nodeid_rules.cpp	2012-10-02 22:52:19 +0000
@@ -600,11 +600,11 @@
           // inherited from the referencing tree if N had been copied into that
           // tree. (On the other hand it is ok if the query result contains nodes
           // which are not shared but have shared descendants). To handle this,
-          // we set theIsInUnsafeContext so that any exprs that (a) extract nodes
+          // we call markInUnsafeContext() so that any exprs that (a) extract nodes
           // out of input nodes and (b) may propagate the extracted nodes to the
           // query result will be considered as unsafe and thus require that 
           // their input trees are standalone.
-          theIsInUnsafeContext = true;
+          markInUnsafeContext(node);
         }
       }
       else
@@ -652,25 +652,11 @@
     expr* node,
     UDFCallChain& udfCaller)
 {
-  TypeManager* tm = node->get_type_manager();
-  RootTypeManager& rtm = GENV_TYPESYSTEM;
-
-  bool savedIsInUnsafeContext = theIsInUnsafeContext;
-
-  if (theIsInUnsafeContext)
-  {
-    xqtref_t retType = node->get_return_type();
-
-    if (TypeOps::is_subtype(tm, *retType, *rtm.ANY_ATOMIC_TYPE_STAR))
-      theIsInUnsafeContext = false;
-  }
-
   switch (node->get_expr_kind())
   {
   case const_expr_kind:
   case var_expr_kind:
   {
-    theIsInUnsafeContext = savedIsInUnsafeContext;
     return;
   }
 
@@ -692,7 +678,8 @@
 
     static_context* sctx = e->get_sctx();
 
-    if (sctx->preserve_mode() != StaticContextConsts::no_preserve_ns)
+    if (sctx->preserve_mode() == StaticContextConsts::preserve_ns &&
+        sctx->inherit_mode() == StaticContextConsts::inherit_ns)
     {
       csize numPairs = e->num_pairs();
       for (csize i = 0; i < numPairs; ++i)
@@ -717,7 +704,8 @@
 
     static_context* sctx = e->get_sctx();
 
-    if (sctx->preserve_mode() != StaticContextConsts::no_preserve_ns)
+    if (sctx->preserve_mode() == StaticContextConsts::preserve_ns &&
+        sctx->inherit_mode() == StaticContextConsts::inherit_ns)
     {
       std::vector<expr*> sources;
       theSourceFinder->findNodeSources(e->get_expr(), &udfCaller, sources);
@@ -732,7 +720,7 @@
   {
     relpath_expr* e = static_cast<relpath_expr *>(node);
 
-    if (theIsInUnsafeContext)
+    if (e->inUnsafeContext())
     {
       std::vector<expr*> sources;
       theSourceFinder->findNodeSources((*e)[0],  &udfCaller, sources);
@@ -750,6 +738,7 @@
 
         if (axisKind != axis_kind_child &&
             axisKind != axis_kind_descendant &&
+            axisKind != axis_kind_descendant_or_self &&
             axisKind != axis_kind_self &&
             axisKind != axis_kind_attribute)
         {
@@ -758,12 +747,24 @@
           markSources(sources);
           break;
         }
+        else
+        {
+          match_expr* matchExpr = axisExpr->getTest();
+
+          if (matchExpr->getTypeName() != NULL &&
+              node->get_sctx()->construction_mode() == StaticContextConsts::cons_strip)
+          {
+            std::vector<expr*> sources;
+            theSourceFinder->findNodeSources((*e)[0],  &udfCaller, sources);
+            markSources(sources);
+            break;
+          }
+        }
       }
     }
 
     applyInternal(rCtx, (*e)[0], udfCaller);
 
-    theIsInUnsafeContext = savedIsInUnsafeContext;
     return;
   }
 
@@ -846,7 +847,9 @@
   {
     if (node->get_sctx()->construction_mode() == StaticContextConsts::cons_strip)
     {
-      theIsInUnsafeContext = true;
+      cast_or_castable_base_expr* e = static_cast<cast_or_castable_base_expr*>(node);
+
+      markInUnsafeContext(e->get_input());
     }
 
     break;
@@ -895,7 +898,8 @@
 
     static_context* sctx = e->get_sctx();
 
-    if (sctx->preserve_mode() != StaticContextConsts::no_preserve_ns)
+    if (sctx->preserve_mode() == StaticContextConsts::preserve_ns &&
+        sctx->inherit_mode() == StaticContextConsts::inherit_ns)
     {
       std::vector<copy_clause*>::const_iterator ite = e->begin();
       std::vector<copy_clause*>::const_iterator end = e->end();
@@ -1028,7 +1032,6 @@
 
     applyInternal(rCtx, udf->getBody(), dummyUdfCaller);
 
-    theIsInUnsafeContext = savedIsInUnsafeContext;
     return;
   }
 
@@ -1049,9 +1052,6 @@
     iter.next();
   }
 
-
-  theIsInUnsafeContext = savedIsInUnsafeContext;
-
   return;
 }
 
@@ -1090,5 +1090,315 @@
 }
 
 
+/*******************************************************************************
+  This method is called when an expr E1 satisfies a condition that may make a
+  sub-expr E2 of E1 be unsafe, even though E2 by itself is safe. 
+
+  This method marks as being in "unsafe context" any expr that may produce
+  nodes which may be propagated into the result of E1.
+********************************************************************************/
+void MarkNodeCopyProps::markInUnsafeContext(expr* node)
+{
+  TypeManager* tm = node->get_type_manager();
+  RootTypeManager& rtm = GENV_TYPESYSTEM;
+
+  xqtref_t retType = node->get_return_type();
+
+  if (TypeOps::is_subtype(tm, *retType, *rtm.ANY_ATOMIC_TYPE_STAR))
+    return;
+
+  switch (node->get_expr_kind())
+  {
+  case const_expr_kind:
+  {
+    return;
+  }
+  case var_expr_kind:
+  {
+    var_expr* e = static_cast<var_expr*>(node);
+
+    switch (e->get_kind())
+    {
+    case var_expr::for_var:
+    case var_expr::let_var:
+    case var_expr::win_var:
+    case var_expr::wincond_out_var:
+    case var_expr::wincond_in_var:
+    case var_expr::non_groupby_var:
+    {
+      if (!e->inUnsafeContext())
+      {
+        e->setInUnsafeContext(ANNOTATION_TRUE);
+        markInUnsafeContext(e->get_domain_expr());
+      }
+      return;
+    }
+
+    case var_expr::copy_var:
+    case var_expr::catch_var:
+    {
+      e->setInUnsafeContext(ANNOTATION_TRUE);
+      return;
+    }
+
+    case var_expr::arg_var:
+    {
+      e->setInUnsafeContext(ANNOTATION_TRUE);
+      return;
+    }
+
+    case var_expr::prolog_var:
+    case var_expr::local_var:
+    {
+      if (!e->inUnsafeContext())
+      {
+        e->setInUnsafeContext(ANNOTATION_TRUE);
+
+        std::vector<expr*>::const_iterator ite = e->setExprsBegin();
+        std::vector<expr*>::const_iterator end = e->setExprsEnd();
+
+        for (; ite != end; ++ite)
+        {
+          expr* setExpr = *ite;
+
+          if (setExpr->get_expr_kind() == var_decl_expr_kind)
+          {
+            markInUnsafeContext(static_cast<var_decl_expr*>(setExpr)->get_init_expr());
+          }
+          else
+          {
+            assert(setExpr->get_expr_kind() == var_set_expr_kind);
+
+            markInUnsafeContext(static_cast<var_set_expr*>(setExpr)->get_expr());
+          }
+        }
+      }
+      return;
+    }
+
+    case var_expr::groupby_var:
+    case var_expr::wincond_in_pos_var:
+    case var_expr::wincond_out_pos_var:
+    case var_expr::pos_var:
+    case var_expr::score_var:
+    case var_expr::count_var:
+    default:
+    {
+      ZORBA_ASSERT(false);
+      return;
+    }
+    }
+  }
+
+  case doc_expr_kind:
+  case elem_expr_kind:
+  case attr_expr_kind:
+  case text_expr_kind:
+  case pi_expr_kind:
+  {
+    break;
+  }
+
+#ifdef ZORBA_WITH_JSON
+  case json_object_expr_kind:
+  case json_direct_object_expr_kind:
+  case json_array_expr_kind:
+  {
+    break;
+  }
+#endif
+
+  case relpath_expr_kind:
+  {
+    relpath_expr* e = static_cast<relpath_expr *>(node);
+    e->setInUnsafeContext(ANNOTATION_TRUE);
+    markInUnsafeContext((*e)[0]);
+    return;
+  }
+
+  case gflwor_expr_kind:
+  case flwor_expr_kind:
+  {
+    flwor_expr* e = static_cast<flwor_expr *>(node);
+    e->setInUnsafeContext(ANNOTATION_TRUE);
+    markInUnsafeContext(e->get_return_expr());
+    return;
+  }
+
+  case if_expr_kind:
+  case trycatch_expr_kind:
+  {
+    break;
+  }
+
+  case fo_expr_kind:
+  {
+    fo_expr* e = static_cast<fo_expr *>(node);
+    function* f = e->get_func();
+
+    e->setInUnsafeContext(ANNOTATION_TRUE);
+
+    if (f->isUdf() && static_cast<user_function*>(f)->getBody() != NULL)
+    {
+      user_function* udf = static_cast<user_function*>(f);
+      expr* body = udf->getBody();
+
+      if (!body->inUnsafeContext())
+      {
+        markInUnsafeContext(body);
+      }
+
+      std::vector<var_expr*>::const_iterator ite = udf->getArgVars().begin();
+      std::vector<var_expr*>::const_iterator end = udf->getArgVars().end();
+      for (; ite != end; ++ite)
+      {
+        expr* argVar = (*ite);
+        if (argVar->inUnsafeContext())
+        {
+          expr* argExpr = e->get_arg(ite - udf->getArgVars().begin());
+          markInUnsafeContext(argExpr);
+        }
+      }
+    } // f->isUdf()
+    else
+    {
+      csize numArgs = e->num_args();
+      for (csize i = 0; i < numArgs; ++i)
+      {
+        if (f->propagatesInputNodes(e, i))
+        {
+          markInUnsafeContext(e->get_arg(i));
+        }
+      }
+    }
+
+    return;
+  }
+
+  case treat_expr_kind:
+  case order_expr_kind:
+  case wrapper_expr_kind:
+  case function_trace_expr_kind:
+  case extension_expr_kind:
+  {
+    break;
+  }
+
+  case validate_expr_kind:
+  {
+    node->setInUnsafeContext(ANNOTATION_TRUE);
+    return;
+  }
+
+  case transform_expr_kind:
+  {
+    transform_expr* e = static_cast<transform_expr *>(node);
+    e->setInUnsafeContext(ANNOTATION_TRUE);
+    markInUnsafeContext(e->getReturnExpr());
+    return;
+  }
+
+  case block_expr_kind:
+  {
+    block_expr* e = static_cast<block_expr *>(node);
+    e->setInUnsafeContext(ANNOTATION_TRUE);
+    expr* lastChild = (*e)[e->size()-1];
+    markInUnsafeContext(lastChild);
+    return;
+  }
+
+  case var_decl_expr_kind:
+  case var_set_expr_kind:
+  {
+    return;
+  }
+
+  case apply_expr_kind:
+  {
+    break;
+  }
+
+  case exit_catcher_expr_kind:
+  {
+    exit_catcher_expr* e = static_cast<exit_catcher_expr*>(node);
+
+    std::vector<expr*>::const_iterator ite = e->exitExprsBegin();
+    std::vector<expr*>::const_iterator end = e->exitExprsEnd();
+
+    for (; ite != end; ++ite)
+    {
+      exit_expr* ex = static_cast<exit_expr*>(*ite);
+
+      markInUnsafeContext(ex->get_expr());
+    }
+
+    break;
+  }
+
+
+  case eval_expr_kind:
+  {
+    break;
+  }
+
+  case debugger_expr_kind:
+  {
+    break;  // ????
+  }
+
+  case dynamic_function_invocation_expr_kind:
+  {
+    break;
+  }
+
+  case function_item_expr_kind:
+  {
+    function_item_expr* e = static_cast<function_item_expr*>(node);
+
+    user_function* udf = static_cast<user_function*>(e->get_function());
+
+    markInUnsafeContext(udf->getBody());
+
+    return;
+  }
+
+  case promote_expr_kind:
+  case castable_expr_kind:
+  case cast_expr_kind:
+  case instanceof_expr_kind:
+  case name_cast_expr_kind:
+  case axis_step_expr_kind:
+  case match_expr_kind:
+  case delete_expr_kind:
+  case rename_expr_kind:
+  case insert_expr_kind:
+  case replace_expr_kind:
+  case while_expr_kind:
+  case flowctl_expr_kind:
+  case exit_expr_kind:
+#ifndef ZORBA_NO_FULL_TEXT
+  case ft_expr_kind:
+#endif
+  default:
+    ZORBA_ASSERT(false);
+  }
+
+  node->setInUnsafeContext(ANNOTATION_TRUE);
+
+  ExprIterator iter(node);
+  while(!iter.done())
+  {
+    expr* child = (**iter);
+    if (child != NULL)
+    {
+      markInUnsafeContext(child);
+    }
+    iter.next();
+  }
+
+  return;
+}
+
+
 }
 /* vim:set et sw=2 ts=2: */
 
=== modified file 'src/compiler/rewriter/rules/ruleset.h'
--- src/compiler/rewriter/rules/ruleset.h	2012-10-02 11:57:15 +0000
+++ src/compiler/rewriter/rules/ruleset.h	2012-10-02 22:52:19 +0000
@@ -125,11 +125,6 @@
 
 /*******************************************************************************
 
-  theIsInUnsafeContext:
-  ---------------------
-  If true, it indicates that an expr E1 satisfies a condition that makes a
-  sub-expr E2 of E1 be unsafe, even though E2 by itself is safe.
-
 ********************************************************************************/
 class MarkNodeCopyProps : public RewriteRule
 {
@@ -140,13 +135,10 @@
 
   UdfCalls         theProcessedUDFCalls;
 
-  bool             theIsInUnsafeContext;
-
 public:
   MarkNodeCopyProps()
     :
-    RewriteRule(RewriteRule::MarkNodeCopyProps, "MarkNodeCopyProps"),
-    theIsInUnsafeContext(false)
+    RewriteRule(RewriteRule::MarkNodeCopyProps, "MarkNodeCopyProps")
   {
   }
 
@@ -156,6 +148,8 @@
   void applyInternal(RewriterContext& rCtx, expr* node, UDFCallChain& udfCaller);
 
   void markSources(const std::vector<expr*>& sources);
+
+  void markInUnsafeContext(expr* node);
 };
 
 
 
=== added file 'test/rbkt/ExpQueryResults/zorba/no-copy/test4.xml.res'
--- test/rbkt/ExpQueryResults/zorba/no-copy/test4.xml.res	1970-01-01 00:00:00 +0000
+++ test/rbkt/ExpQueryResults/zorba/no-copy/test4.xml.res	2012-10-02 22:52:19 +0000
@@ -0,0 +1,1 @@
+<s:name xmlns:s="http://www.zorba-xquery.org/simple">foo</s:name>
 
=== added file 'test/rbkt/ExpQueryResults/zorba/no-copy/test5.xml.res'
--- test/rbkt/ExpQueryResults/zorba/no-copy/test5.xml.res	1970-01-01 00:00:00 +0000
+++ test/rbkt/ExpQueryResults/zorba/no-copy/test5.xml.res	2012-10-02 22:52:19 +0000
@@ -0,0 +1,1 @@
+<s:name xmlns:s="http://www.zorba-xquery.org/simple">foo</s:name>    true
 
=== added file 'test/rbkt/Queries/zorba/no-copy/test4.xq'
--- test/rbkt/Queries/zorba/no-copy/test4.xq	1970-01-01 00:00:00 +0000
+++ test/rbkt/Queries/zorba/no-copy/test4.xq	2012-10-02 22:52:19 +0000
@@ -0,0 +1,17 @@
+
+declare namespace opt = "http://www.zorba-xquery.com/options/optimizer";
+
+declare construction strip;
+
+declare copy-namespaces preserve, no-inherit;
+
+
+import schema namespace s="http://www.zorba-xquery.org/simple" at "simple.xsd";
+
+declare option opt:enable "for-serialization-only";
+
+declare variable $doc := <s:person><s:name>foo</s:name><s:age>25</s:age></s:person>;
+
+let $vdoc := validate { $doc }
+let $copy := <root>{$vdoc/s:name}</root>
+return $copy/child::element(*, xs:untyped)
 
=== added file 'test/rbkt/Queries/zorba/no-copy/test5.xq'
--- test/rbkt/Queries/zorba/no-copy/test5.xq	1970-01-01 00:00:00 +0000
+++ test/rbkt/Queries/zorba/no-copy/test5.xq	2012-10-02 22:52:19 +0000
@@ -0,0 +1,16 @@
+declare namespace opt = "http://www.zorba-xquery.com/options/optimizer";
+
+declare construction strip;
+
+declare copy-namespaces preserve, no-inherit;
+
+import schema namespace s="http://www.zorba-xquery.org/simple" at "simple.xsd";
+
+declare option opt:enable "for-serialization-only";
+
+declare variable $doc := <s:person><s:name>foo</s:name><s:age>25</s:age></s:person>;
+
+let $vdoc := validate { $doc }
+let $copy := <root>{$vdoc/s:name}</root>
+let $copyname := $copy/s:name
+return ($copyname, "   ", $copyname instance of element(*, xs:string))