2005-05-30 Frank Ch. Eigler <fche@redhat.com>

	More fully parse & elaborate "expr in array" construct.
	* staptree.h (array_in): Make this unary.  Update .cxx to match.
	* parse.cxx (parse_array_in): Rewrite.
	(parse_symbol_plain): Removed.  Update .h to match.
	* elaborate.cxx (typeresolution_info::visit_array_in): New function.
	(find_array): Tentatively, accept arity=0.
	* translate.cxx (c_unparser::c_assign): New functions to eliminate
	much ugly duplication.  Use throughout.
	(visit_symbol): Correct function formal argument search.
	(c_tmpcounter*::visit): Add missing recursion in several functions.
	* testsuite/*: Add new tests for array-in construct.  Add the
	first "transok" test.
	* Makefile.am: Add transok tests.
	* Makefile.in: Regenerated.

15 files changed, 305 insertions, 128 deletions

diff --git a/ChangeLog b/ChangeLog
index 63512a0b..41576ba4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2005-05-30  Frank Ch. Eigler  <fche@redhat.com>
+	
+	More fully parse & elaborate "expr in array" construct.
+	* staptree.h (array_in): Make this unary.  Update .cxx to match.
+	* parse.cxx (parse_array_in): Rewrite.
+	(parse_symbol_plain): Removed.  Update .h to match.
+	* elaborate.cxx (typeresolution_info::visit_array_in): New function.
+	(find_array): Tentatively, accept arity=0.
+	* translate.cxx (c_unparser::c_assign): New functions to eliminate
+	much ugly duplication.  Use throughout.
+	(visit_symbol): Correct function formal argument search.
+	(c_tmpcounter*::visit): Add missing recursion in several functions.
+	* testsuite/*: Add new tests for array-in construct.  Add the
+	first "transok" test.
+	* Makefile.am: Add transok tests.
+	* Makefile.in: Regenerated.
+
 2005-05-26  Frank Ch. Eigler  <fche@redhat.com>
 
 	* translate.cxx: Traverse trees just for common-header generation,
diff --git a/Makefile.am b/Makefile.am
index e397e2ad..b2eb770c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -21,9 +21,13 @@ DEFS= -DDEFPATH=$(DEFPATH) -DHAVE_CONFIG_H
 
 p=$(srcdir)/testsuite/parse
 s=$(srcdir)/testsuite/sem
+t=$(srcdir)/testsuite/trans
 TESTS = $(wildcard $(p)ok/*.stp) $(wildcard $(p)ko/*.stp) \
-	$(wildcard $(s)ok/*.stp) $(wildcard $(s)ko/*.stp)
-XFAIL_TESTS = $(wildcard $(p)ko/*.stp) $(wildcard $(s)ko/*.stp)
+	$(wildcard $(s)ok/*.stp) $(wildcard $(s)ko/*.stp) \
+	$(wildcard $(t)ok/*.stp) $(wildcard $(t)ko/*.stp)
+XFAIL_TESTS = $(wildcard $(p)ko/*.stp) \
+	$(wildcard $(s)ko/*.stp) \
+	$(wildcard $(t)ko/*.stp)
 
 TESTS_ENVIRONMENT = $(srcdir)/runtest.sh
 
diff --git a/Makefile.in b/Makefile.in
index b3499a3e..efaaa1b5 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -184,10 +184,15 @@ stapdatadir = @datadir@/systemtap
 DEFPATH = "\".$(PATH_SEPARATOR)$(stapdatadir)\""
 p = $(srcdir)/testsuite/parse
 s = $(srcdir)/testsuite/sem
+t = $(srcdir)/testsuite/trans
 TESTS = $(wildcard $(p)ok/*.stp) $(wildcard $(p)ko/*.stp) \
-	$(wildcard $(s)ok/*.stp) $(wildcard $(s)ko/*.stp)
+	$(wildcard $(s)ok/*.stp) $(wildcard $(s)ko/*.stp) \
+	$(wildcard $(t)ok/*.stp) $(wildcard $(t)ko/*.stp)
+
+XFAIL_TESTS = $(wildcard $(p)ko/*.stp) \
+	$(wildcard $(s)ko/*.stp) \
+	$(wildcard $(t)ko/*.stp)
 
-XFAIL_TESTS = $(wildcard $(p)ko/*.stp) $(wildcard $(s)ko/*.stp)
 TESTS_ENVIRONMENT = $(srcdir)/runtest.sh
 all: config.h
 	$(MAKE) $(AM_MAKEFLAGS) all-am
diff --git a/elaborate.cxx b/elaborate.cxx
index cac773bf..02c81a2d 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -220,7 +220,8 @@ symresolution_info::visit_symbol (symbol* e)
       else if (current_probe)
         current_probe->locals.push_back (v);
       else
-        throw semantic_error ("no current probe/function for unresolved scalar", e->tok);
+        // must not happen
+        throw semantic_error ("no current probe/function", e->tok);
       e->referent = v;
     }
 }
@@ -340,10 +341,11 @@ symresolution_info::find_array (const string& name, unsigned arity)
   // search processed globals
   for (unsigned i=0; i<session.globals.size(); i++)
     if (session.globals[i]->name == name)
-      if ((session.globals[i]->arity == (int) arity) ||
+      if ((arity > 0 && (session.globals[i]->arity == (int) arity)) ||
 	  session.globals[i]->arity < 0)
 	{
-	  session.globals[i]->set_arity (arity);
+          if (arity > 0)
+            session.globals[i]->set_arity (arity);
 	  return session.globals[i];
 	}
 
@@ -353,10 +355,11 @@ symresolution_info::find_array (const string& name, unsigned arity)
       stapfile* f = session.library_files[i];
       for (unsigned j=0; j<f->globals.size(); j++)
         if (f->globals[j]->name == name)
-          if ((f->globals[j]->arity == (int) arity) ||
+          if ((arity > 0 && (f->globals[j]->arity == (int) arity)) ||
               f->globals[j]->arity < 0)
           {
-            f->globals[j]->set_arity (arity);
+            if (arity > 0)
+              f->globals[j]->set_arity (arity);
 
             // put library into the queue if not already there
             if (0) // (session.verbose_resolution)
@@ -925,8 +928,20 @@ typeresolution_info::visit_delete_statement (delete_statement* e)
 void
 typeresolution_info::visit_array_in (array_in* e)
 {
-  // XXX: not yet supported
-  unresolved (e->tok);
+  // all unary operators only work on numerics
+  exp_type t1 = t;
+  t = pe_unknown; // array value can be anything
+  e->operand->visit (this);
+
+  if (t1 == pe_unknown && e->type != pe_unknown)
+    ; // already resolved
+  else if (t1 == pe_string || t1 == pe_stats)
+    mismatch (e->tok, t1, pe_long);
+  else if (e->type == pe_unknown)
+    {
+      e->type = pe_long;
+      resolved (e->tok, e->type);
+    }
 }
 
 
diff --git a/parse.cxx b/parse.cxx
index 2bf0f977..5f3d6971 100644
--- a/parse.cxx
+++ b/parse.cxx
@@ -830,21 +830,67 @@ parser::parse_logical_and ()
 expression*
 parser::parse_array_in ()
 {
-  expression* op1 = parse_comparison ();
+  // This is a very tricky case.  All these are legit expressions:
+  // "a in b"  "a+0 in b" "(a,b) in c" "(c,(d+0)) in b"
+  vector<expression*> indexes;
+  bool parenthesized = false;
 
   const token* t = peek ();
+  if (t && t->type == tok_operator && t->content == "(")
+    {
+      next ();
+      parenthesized = true;
+    }
+
+  while (1)
+    {
+      expression* op1 = parse_comparison ();
+      indexes.push_back (op1);
+
+      if (parenthesized)
+        {
+          const token* t = peek ();
+          if (t && t->type == tok_operator && t->content == ",")
+            {
+              next ();
+              continue;
+            }
+          else if (t && t->type == tok_operator && t->content == ")")
+            {
+              next ();
+              break;
+            }
+          else 
+            throw parse_error ("expected ',' or ')'");
+        }
+      else
+        break; // expecting only one expression
+    }
+
+  t = peek ();
   if (t && t->type == tok_identifier && t->content == "in")
     {
       array_in *e = new array_in;
-      e->left = op1;
       e->op = t->content;
       e->tok = t;
-      next ();
-      e->right = parse_symbol_plain ();
+      next (); // swallow "in"
+
+      arrayindex* a = new arrayindex;
+      a->indexes = indexes;
+
+      t = next ();
+      if (t->type != tok_identifier)
+        throw parse_error ("expected identifier");
+      a->tok = t;
+      a->base = t->content;
+
+      e->operand = a;
       return e;
     }
+  else if (indexes.size() == 1) // no "in" - need one expression only
+    return indexes[0];
   else
-    return op1;
+    throw parse_error ("unexpected comma-separated expression list");
 }
 
 
@@ -1132,15 +1178,3 @@ parser::parse_symbol ()
     }
 }
 
-
-symbol*
-parser::parse_symbol_plain () // var only
-{
-  symbol *s = new symbol;
-  const token* t = next ();
-  if (t->type != tok_identifier)
-    throw parse_error ("expected identifier");
-  s->name = t->content;
-  s->tok = t;
-  return s;
-}
diff --git a/parse.h b/parse.h
index c0e457e9..3ccb58fe 100644
--- a/parse.h
+++ b/parse.h
@@ -113,7 +113,6 @@ private: // nonterminals
   expression* parse_crement ();
   expression* parse_value ();
   expression* parse_symbol ();
-  symbol* parse_symbol_plain ();
 };
 
 
diff --git a/staptree.cxx b/staptree.cxx
index b1130d94..cf934fa2 100644
--- a/staptree.cxx
+++ b/staptree.cxx
@@ -630,8 +630,7 @@ traversing_visitor::visit_logical_and_expr (logical_and_expr* e)
 void
 traversing_visitor::visit_array_in (array_in* e)
 {
-  e->left->visit (this);
-  e->right->visit (this);
+  e->operand->visit (this);
 }
 
 void
diff --git a/staptree.h b/staptree.h
index 203bc27e..2d2c9fce 100644
--- a/staptree.h
+++ b/staptree.h
@@ -124,7 +124,7 @@ struct logical_and_expr: public binary_expression
 };
 
 
-struct array_in: public binary_expression
+struct array_in: public unary_expression
 {
   void visit (visitor* u);
 };
diff --git a/testsuite/parseko/seven.stp b/testsuite/parseko/seven.stp
new file mode 100755
index 00000000..06f8a3ba
--- /dev/null
+++ b/testsuite/parseko/seven.stp
@@ -0,0 +1,8 @@
+#! stap -p1
+
+probe foo {
+  (a,) in b;
+  (,c) in d;
+  () in e;
+  a in e[2];
+}
diff --git a/testsuite/parseok/eight.stp b/testsuite/parseok/eight.stp
new file mode 100755
index 00000000..0da5c8d2
--- /dev/null
+++ b/testsuite/parseok/eight.stp
@@ -0,0 +1,6 @@
+#! stap -p1
+
+probe all
+{
+  "1" in a1;  ("1", 2) in a2;  (a) in a3;
+}
diff --git a/testsuite/semko/ten.stp b/testsuite/semko/ten.stp
new file mode 100755
index 00000000..3e233228
--- /dev/null
+++ b/testsuite/semko/ten.stp
@@ -0,0 +1,5 @@
+#! stap -p2
+
+global foo
+probe p1 { foo = 1 }
+probe p2 { if (4 in foo) { } }
diff --git a/testsuite/semok/seven.stp b/testsuite/semok/seven.stp
index 8351c1e2..b9ce8cb9 100755
--- a/testsuite/semok/seven.stp
+++ b/testsuite/semok/seven.stp
@@ -7,7 +7,7 @@ function printk (s) { str=s.""; return 0 } # to become a built-in
 
 function search (key) 
 {
-  if (1) # (key in ar1)
+  if (key in ar1)
     { ar1[key] ++; return ar2[ar1[key]] }
   else
     return "no can do"  # implies ar2[]: string
@@ -22,6 +22,6 @@ probe syscall("zamboni")
 probe end
 {
   # for (key in ar2)
-  if (1) # (key in ar2)
+  if (key in ar2)
     printk ("this: " . string (key) . " was " . ar2[key])
 }
diff --git a/testsuite/semok/ten.stp b/testsuite/semok/ten.stp
new file mode 100755
index 00000000..d56c2a4a
--- /dev/null
+++ b/testsuite/semok/ten.stp
@@ -0,0 +1,10 @@
+#! stap -p1
+
+global a1, a2, a3
+
+probe all
+{
+  a = "1" in a1;
+  a = ("1", a) in a2;  
+  a = (a, a+a, a1[a], a2[0+a]) in a3;
+}
diff --git a/testsuite/transok/three.stp b/testsuite/transok/three.stp
new file mode 100755
index 00000000..ce94531c
--- /dev/null
+++ b/testsuite/transok/three.stp
@@ -0,0 +1,27 @@
+#! stap
+
+function f1 (a, b) {
+  c = 1;
+  d = "hello";
+  # poo[c] = bab[d] = "hi"
+  bab = "hi";
+  bab = poo[c];
+  return 0
+} 
+
+function f2 () {
+  return f1 (4, "zoo");
+}
+
+global koo
+global poo, bab
+
+probe z {
+  f2 ();
+  koo = 1
+}
+
+probe x,y {
+  f2 ();
+  f1 (f1 (3, "foo"), "canoe")
+}
diff --git a/translate.cxx b/translate.cxx
index 8c17c7cb..ebfd619e 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -7,10 +7,25 @@
 #include "elaborate.h"
 #include "translate.h"
 #include <iostream>
+#include <sstream>
 
 using namespace std;
 
 
+
+// little utility function
+
+template <typename T>
+static string
+stringify(T t)
+{
+  ostringstream s;
+  s << t;
+  return s.str ();
+}
+
+
+
 // ------------------------------------------------------------------------
 // toy provider/unparser pair
 
@@ -66,6 +81,9 @@ struct c_unparser: public unparser, public visitor
 
   string c_typename (exp_type e);
   string c_varname (const string& e);
+  void c_assign (const string& lvalue, expression* rvalue, const string& msg);
+  void c_assign (const string& lvalue, const string& rvalue, exp_type type,
+                 const string& msg, const token* tok);
 
   void visit_block (block *s);
   void visit_null_statement (null_statement *s);
@@ -410,6 +428,7 @@ c_unparser::emit_function (functiondecl* v)
   this->current_probe = 0;
   this->current_probenum = 0;
   this->current_function = v;
+  this->tmpvar_counter = 0;
 
   o->newline()
     << "struct function_" << c_varname (v->name) << "_locals * "
@@ -480,6 +499,7 @@ c_unparser::emit_probe (derived_probe* v, unsigned i)
   this->current_function = 0;
   this->current_probe = v;
   this->current_probenum = i;
+  this->tmpvar_counter = 0;
   v->body->visit (this);
   this->current_probe = 0;
   this->current_probenum = 0; // not essential
@@ -518,6 +538,51 @@ c_unparser::c_varname (const string& e)
 }
 
 
+void
+c_unparser::c_assign (const string& lvalue, expression* rvalue,
+                      const string& msg)
+{
+  if (rvalue->type == pe_long)
+    {
+      o->newline() << lvalue << " = ";
+      rvalue->visit (this);
+      o->line() << ";";
+    }
+  else if (rvalue->type == pe_string)
+    {
+      o->newline() << "strncpy (" << lvalue << ", ";
+      rvalue->visit (this);
+      o->line() << ", MAXSTRINGLEN);";
+    }
+  else
+    {
+      string fullmsg = msg + " type unsupported";
+      throw semantic_error (fullmsg, rvalue->tok);
+    }
+}
+
+
+void
+c_unparser::c_assign (const string& lvalue, const string& rvalue,
+                      exp_type type, const string& msg, const token* tok)
+{
+  if (type == pe_long)
+    {
+      o->newline() << lvalue << " = " << rvalue << ";";
+    }
+  else if (type == pe_string)
+    {
+      o->newline() << "strncpy (" << lvalue << ", "
+                   << rvalue << ", MAXSTRINGLEN);";
+    }
+  else
+    {
+      string fullmsg = msg + " type unsupported";
+      throw semantic_error (fullmsg, tok);
+    }
+}
+
+
 
 void
 c_unparser::visit_block (block *s)
@@ -624,21 +689,7 @@ c_unparser::visit_return_statement (return_statement* s)
                          "vs", s->tok);
 
   o->newline() << "/* " << *s->tok << " */";
-  if (s->value->type == pe_long)
-    {
-      o->newline() << "l->__retvalue = ";
-      s->value->visit (this);
-      o->line() << ";";
-    }
-  else if (s->value->type == pe_string)
-    {
-      o->newline() << "strncpy (l->__retvalue, ";
-      s->value->visit (this);
-      o->line() << ", MAXSTRINGLEN);";
-      o->line() << ";";
-    }
-  else
-    throw semantic_error ("return type unsupported", s->tok);
+  c_assign ("l->__retvalue", s->value, "return value");
 
   o->newline() << "goto out;";
 }
@@ -820,6 +871,7 @@ c_unparser::visit_symbol (symbol* e)
     }
   else if (current_function)
     {
+      // check locals
       for (unsigned i=0; i<current_function->locals.size(); i++)
 	{
 	  vardecl* rr = current_function->locals[i];
@@ -829,6 +881,17 @@ c_unparser::visit_symbol (symbol* e)
 	      return;
 	    }
 	}
+
+      // check formal args
+      for (unsigned i=0; i<current_function->formal_args.size(); i++)
+	{
+	  vardecl* rr = current_function->formal_args[i];
+	  if (rr == r) // comparison of pointers is sufficient
+	    {
+	      o->line() << "l->" << c_varname (r->name);
+	      return;
+	    }
+	}
     }
 
   // it better be a global
@@ -850,8 +913,11 @@ c_unparser::visit_symbol (symbol* e)
 void
 c_tmpcounter_assignment::visit_symbol (symbol *e)
 {
-  parent->parent->o->newline() << parent->parent->c_typename (e->type)
-                               << " __tmp" << parent->tmpvar_counter ++ << ";";
+  parent->parent->o->newline()
+    << parent->parent->c_typename (e->type)
+    << " __tmp" << parent->tmpvar_counter ++ << ";"
+    << " /* " << e->name << " rvalue */";
+  rvalue->visit (parent);
 }
 
 
@@ -883,16 +949,7 @@ c_unparser_assignment::visit_symbol (symbol *e)
   o->line() << "({ ";
   o->indent(1);
 
-  if (e->type == pe_string)
-    o->newline() << "strncpy (";
-  else
-    o->newline();
-  o->line() << tmp_base << tmpidx;
-  o->line() << (e->type == pe_long ? " = " : ", ");
-  rvalue->visit (parent);
-  if (e->type == pe_string)
-    o->line() << ", MAXSTRINGLEN)";
-  o->line() << ";";
+  parent->c_assign (tmp_base + stringify (tmpidx), rvalue, "assignment");
 
   // XXX: strings may be passed safely via a char*, without
   // a full copy in tmpNNN
@@ -905,16 +962,11 @@ c_unparser_assignment::visit_symbol (symbol *e)
 	  vardecl* rr = current_probe->locals[i];
 	  if (rr == r) // comparison of pointers is sufficient
 	    {
-	      if (e->type == pe_string)
-		o->newline() << "strncpy (";
-	      else
-		o->newline();
-	      o->line() << "l->" << parent->c_varname (r->name);
-	      o->line() << (e->type == pe_long ? " = " : ", ")
-			<< tmp_base << tmpidx;
-	      if (e->type == pe_string)
-		o->line() << ", MAXSTRINGLEN)";
-	      o->line() << ";";
+              parent->c_assign ("l->" + parent->c_varname (r->name),
+                                tmp_base + stringify (tmpidx),
+                                rvalue->type,
+                                "local variable assignment", rvalue->tok); 
+
               o->newline() << tmp_base << tmpidx << ";";
 	      o->newline(-1) << "})";
 	      return;
@@ -928,16 +980,27 @@ c_unparser_assignment::visit_symbol (symbol *e)
 	  vardecl* rr = current_function->locals[i];
 	  if (rr == r) // comparison of pointers is sufficient
 	    {
-	      if (e->type == pe_string)
-		o->newline() << "strncpy (";
-	      else
-		o->newline();
-	      o->line() << "l->" << parent->c_varname (r->name);
-	      o->line() << (e->type == pe_long ? " = " : ", ")
-			<< tmp_base << tmpidx;
-	      if (e->type == pe_string)
-		o->line() << ", MAXSTRINGLEN)";
-	      o->line() << ";";
+              parent->c_assign ("l->" + parent->c_varname (r->name),
+                                tmp_base + stringify (tmpidx),
+                                rvalue->type,
+                                "local variable assignment", rvalue->tok); 
+
+	      o->newline() << tmp_base << tmpidx << ";";
+	      o->newline(-1) << "})";
+	      return;
+	    }
+	}
+
+      for (unsigned i=0; i<current_function->formal_args.size(); i++)
+	{
+	  vardecl* rr = current_function->formal_args[i];
+	  if (rr == r) // comparison of pointers is sufficient
+	    {
+              parent->c_assign ("l->" + parent->c_varname (r->name),
+                                tmp_base + stringify (tmpidx),
+                                rvalue->type,
+                                "formal argument assignment", rvalue->tok); 
+
 	      o->newline() << tmp_base << tmpidx << ";";
 	      o->newline(-1) << "})";
 	      return;
@@ -951,16 +1014,12 @@ c_unparser_assignment::visit_symbol (symbol *e)
       if (session->globals[i] == r)
 	{
 	  // XXX: acquire write lock on global
-	  if (e->type == pe_string)
-	    o->newline() << "strncpy (";
-	  else
-	    o->newline();
-	  o->line() << "global_" << parent->c_varname (r->name);
-	  o->line() << (e->type == pe_long ? " = " : ", ")
-		    << tmp_base << tmpidx;
-	  if (e->type == pe_string)
-	    o->line() << ", MAXSTRINGLEN)";
-	  o->line() << ";";
+
+          parent->c_assign ("global_" + parent->c_varname (r->name),
+                            tmp_base + stringify (tmpidx),
+                            rvalue->type,
+                            "global variable assignment", rvalue->tok); 
+
 	  o->newline() << tmp_base << tmpidx << ";";
 	  o->newline(-1) << "})";
 	  return;
@@ -979,11 +1038,16 @@ c_tmpcounter::visit_arrayindex (arrayindex *e)
   for (unsigned i=0; i<r->index_types.size(); i++)
     parent->o->newline()
       << parent->c_typename (r->index_types[i])
-      << " __tmp" << tmpvar_counter ++ << ";";
+      << " __tmp" << tmpvar_counter ++ << ";"
+      << " /* " << e->base << " idx #" << i << " */";
   // now the result
   parent->o->newline()
     << parent->c_typename (r->type)
-    << " __tmp" << tmpvar_counter ++ << ";";
+    << " __tmp" << tmpvar_counter ++ << ";"
+    << " /* " << e->base << "value */";
+
+  for (unsigned i=0; i<e->indexes.size(); i++)
+    e->indexes[i]->visit (this);
 }
 
 
@@ -1020,16 +1084,9 @@ c_unparser::visit_arrayindex (arrayindex* e)
       throw semantic_error ("array index type mismatch", e->indexes[i]->tok);
 
     unsigned tmpidx = tmpidx_base + i;
-    if (e->indexes[i]->type == pe_string)
-      o->newline() << "strncpy (";
-    else
-      o->newline();
-    o->line() << tmp_base << tmpidx;
-    o->line() << (e->indexes[i]->type == pe_long ? " = " : ", ");
-    e->indexes[i]->visit (this);
-    if (e->indexes[i]->type == pe_string)
-      o->line() << ", MAXSTRINGLEN)";
-    o->line() << ";";
+
+    c_assign (tmp_base + stringify (tmpidx),
+              e->indexes[i], "array index copy");
     }
 
   o->newline() << "if (errorcount)";
@@ -1078,7 +1135,11 @@ c_tmpcounter::visit_functioncall (functioncall *e)
   for (unsigned i=0; i<r->formal_args.size(); i++)
     parent->o->newline()
       << parent->c_typename (r->formal_args[i]->type)
-      << " __tmp" << tmpvar_counter ++ << ";";
+      << " __tmp" << tmpvar_counter ++ << ";"
+      << " /* " << e->function << " arg #" << i << " */";
+
+  for (unsigned i=0; i<e->args.size(); i++)
+    e->args[i]->visit (this);
 }
 
 
@@ -1110,17 +1171,9 @@ c_unparser::visit_functioncall (functioncall* e)
       if (r->formal_args[i]->type != e->args[i]->type)
 	throw semantic_error ("function argument type mismatch",
 			      e->args[i]->tok, "vs", r->formal_args[i]->tok);
-      
-      if (e->args[i]->type == pe_string)
-	o->newline() << "strncpy (";
-      else
-	o->newline();
-      o->line() << tmp_base << tmpidx;
-      o->line() << (e->args[i]->type == pe_long ? " = " : ", ");
-      e->args[i]->visit (this);
-      if (e->args[i]->type == pe_string)
-	o->line() << ", MAXSTRINGLEN)";
-      o->line() << ";";
+
+      c_assign (tmp_base + stringify(tmpidx),
+                e->args[i], "function actual argument evaluation");
     }
 
   o->newline() << "if (c->nesting+2 >= MAXNESTING)";
@@ -1140,19 +1193,14 @@ c_unparser::visit_functioncall (functioncall* e)
       if (r->formal_args[i]->type != e->args[i]->type)
 	throw semantic_error ("function argument type mismatch",
 			      e->args[i]->tok, "vs", r->formal_args[i]->tok);
-      
-      if (e->args[i]->type == pe_string)
-	o->newline() << "strncpy (";
-      else
-	o->newline();
-      o->line() << "c->locals[c->nesting+1]"
-		<< ".function_" << c_varname (r->name)
-		<< "." << c_varname (r->formal_args[i]->name);
-      o->line() << (e->args[i]->type == pe_long ? " = " : ", ");
-      o->line() << tmp_base << tmpidx;
-      if (e->args[i]->type == pe_string)
-	o->line() << ", MAXSTRINGLEN)";
-      o->line() << ";";
+
+      c_assign ("c->locals[c->nesting+1].function_" +
+		c_varname (r->name) + "." +
+                c_varname (r->formal_args[i]->name),
+                tmp_base + stringify (tmpidx),
+                e->args[i]->type,
+                "function actual argument copy",
+                e->args[i]->tok);
     }
 
   // call function