Mercurial > octave

changeset 28257:ae94e3fad6d4 stable

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
fix printing of functions with varargin/varargout (bug #58279) * pt-misc.h, pt-misc.cc (tree_parameter_list::m_in_or_out): New data member. (tree_parameter_list::varargs_symbol_name, tree_parameter_list::is_input_list, tree_parameter_list::is_output_list): New functions. (tree_parameter_list::variable_names): Also include varargin or varargout in the list. (tree_parameter_list::dup): Pass m_in_or_out to tree_parameter_list constructor. * oct-parse.yy (opt_param_list): Don't create tree_parameter_list object for empty lists that have no parens. (param_list1): Always create a tree_parameter_list object. * pt-pr-code.cc (tree_print_code::visit_octave_user_function_header): Don't print input/output parameter lists here. (tree_print_code::visit_anon_fcn_handle): Don't print parens for parameter list here. (tree_print_code::visit_parameter_list): Handle printing of all parts of parameter lists here, including parens or brackets. For output lists, skip brackets if there is a single element in the list, including varargout. * ov-fcn-handle.cc (octave_fcn_handle::print_raw): Don't print parens for parameter list here.
author John W. Eaton <jwe@octave.org>
date Fri, 01 May 2020 00:51:01 -0400
parents 3241ede9806c
children e9126e73748a ec77c790fce2
files libinterp/octave-value/ov-fcn-handle.cc libinterp/parse-tree/oct-parse.yy libinterp/parse-tree/pt-misc.cc libinterp/parse-tree/pt-misc.h libinterp/parse-tree/pt-pr-code.cc
diffstat 5 files changed, 88 insertions(+), 68 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/octave-value/ov-fcn-handle.cc	Thu Apr 30 23:51:30 2020 -0400
+++ b/libinterp/octave-value/ov-fcn-handle.cc	Fri May 01 00:51:01 2020 -0400
@@ -1654,14 +1654,18 @@
 
       if (f)
         {
-          octave::tree_parameter_list *p = f->parameter_list ();
+          os << "@";
 
-          os << "@(";
+          // The parameter list should always be valid for anonymous
+          // functions, so we should always call accept for it, and it
+          // will print the parens for us.
+
+          octave::tree_parameter_list *p = f->parameter_list ();
 
           if (p)
             p->accept (tpc);
 
-          os << ") ";
+          os << " ";
 
           octave::tree_statement_list *b = f->body ();
 
--- a/libinterp/parse-tree/oct-parse.yy	Thu Apr 30 23:51:30 2020 -0400
+++ b/libinterp/parse-tree/oct-parse.yy	Fri May 01 00:51:01 2020 -0400
@@ -1429,7 +1429,7 @@
                 ;
 
 param_list1     : // empty
-                  { $$ = nullptr; }
+                  { $$ = new octave::tree_parameter_list (octave::tree_parameter_list::in); }
                 | param_list2
                   {
                     $1->mark_as_formal_parameters ();
@@ -1448,7 +1448,7 @@
                 ;
 
 param_list2     : param_list_elt
-                  { $$ = new octave::tree_parameter_list ($1); }
+                  { $$ = new octave::tree_parameter_list (octave::tree_parameter_list::in, $1); }
                 | param_list2 ',' param_list_elt
                   {
                     YYUSE ($2);
@@ -1475,13 +1475,14 @@
 
                     lexer.m_looking_at_return_list = false;
 
-                    $$ = new octave::tree_parameter_list ();
+                    $$ = new octave::tree_parameter_list (octave::tree_parameter_list::out);
                   }
                 | identifier
                   {
                     lexer.m_looking_at_return_list = false;
 
-                    octave::tree_parameter_list *tmp = new octave::tree_parameter_list ($1);
+                    octave::tree_parameter_list *tmp
+                      = new octave::tree_parameter_list (octave::tree_parameter_list::out, $1);
 
                     // Even though this parameter list can contain only
                     // a single identifier, we still need to validate it
@@ -1516,7 +1517,9 @@
                 ;
 
 return_list1    : identifier
-                  { $$ = new octave::tree_parameter_list (new octave::tree_decl_elt ($1)); }
+                  {
+                    $$ = new octave::tree_parameter_list (octave::tree_parameter_list::out, new octave::tree_decl_elt ($1));
+                  }
                 | return_list1 ',' identifier
                   {
                     YYUSE ($2);
--- a/libinterp/parse-tree/pt-misc.cc	Thu Apr 30 23:51:30 2020 -0400
+++ b/libinterp/parse-tree/pt-misc.cc	Fri May 01 00:51:01 2020 -0400
@@ -59,13 +59,16 @@
     for (tree_decl_elt *elt : *this)
       retval.push_back (elt->name ());
 
+    if (m_marked_for_varargs)
+      retval.push_back (varargs_symbol_name ());
+
     return retval;
   }
 
   tree_parameter_list *
   tree_parameter_list::dup (symbol_scope& scope) const
   {
-    tree_parameter_list *new_list = new tree_parameter_list ();
+    tree_parameter_list *new_list = new tree_parameter_list (m_in_or_out);
 
     new_list->m_marked_for_varargs = m_marked_for_varargs;
 
--- a/libinterp/parse-tree/pt-misc.h	Thu Apr 30 23:51:30 2020 -0400
+++ b/libinterp/parse-tree/pt-misc.h	Fri May 01 00:51:01 2020 -0400
@@ -52,14 +52,21 @@
       out = 2
     };
 
-    tree_parameter_list (void)
-      : m_marked_for_varargs (0) { }
+    tree_parameter_list (in_or_out io)
+      : m_in_or_out (io), m_marked_for_varargs (0)
+    { }
 
-    tree_parameter_list (tree_decl_elt *t)
-      : m_marked_for_varargs (0) { append (t); }
+    tree_parameter_list (in_or_out io, tree_decl_elt *t)
+      : m_in_or_out (io), m_marked_for_varargs (0)
+    {
+      append (t);
+    }
 
-    tree_parameter_list (tree_identifier *id)
-      : m_marked_for_varargs (0) { append (new tree_decl_elt (id)); }
+    tree_parameter_list (in_or_out io, tree_identifier *id)
+      : m_in_or_out (io), m_marked_for_varargs (0)
+    {
+      append (new tree_decl_elt (id));
+    }
 
     // No copying!
 
@@ -79,8 +86,17 @@
 
     bool varargs_only (void) { return (m_marked_for_varargs < 0); }
 
+    bool is_input_list (void) const { return m_in_or_out == in; }
+
+    bool is_output_list (void) const { return m_in_or_out == out; }
+
     std::list<std::string> variable_names (void) const;
 
+    std::string varargs_symbol_name (void) const
+    {
+      return m_in_or_out == in ? "varargin" : "varargout";
+    }
+
     tree_parameter_list * dup (symbol_scope& scope) const;
 
     void accept (tree_walker& tw)
@@ -90,6 +106,11 @@
 
   private:
 
+    in_or_out m_in_or_out;
+
+    // 1: takes varargs
+    // -1: takes varargs only
+    // 0: does not take varargs.
     int m_marked_for_varargs;
   };
 }
--- a/libinterp/parse-tree/pt-pr-code.cc	Thu Apr 30 23:51:30 2020 -0400
+++ b/libinterp/parse-tree/pt-pr-code.cc	Fri May 01 00:51:01 2020 -0400
@@ -44,15 +44,13 @@
 
     print_parens (afh, "(");
 
-    m_os << "@(";
+    m_os << "@";
 
     tree_parameter_list *param_list = afh.parameter_list ();
 
     if (param_list)
       param_list->accept (*this);
 
-    m_os << ") ";
-
     print_fcn_handle_body (afh.expression ());
 
     print_parens (afh, ")");
@@ -345,35 +343,10 @@
 
     if (ret_list)
       {
-        bool takes_var_return = fcn.takes_var_return ();
-
-        int len = ret_list->length ();
-
-        if (len > 1 || takes_var_return)
-          {
-            m_os << '[';
-            m_nesting.push ('[');
-          }
-
         ret_list->accept (*this);
 
-        if (takes_var_return)
-          {
-            if (len > 0)
-              m_os << ", ";
-
-            m_os << "varargout";
-          }
-
-        if (len > 1 || takes_var_return)
-          {
-            m_nesting.pop ();
-            m_os << ']';
-          }
-
         m_os << " = ";
       }
-
     std::string fcn_name = fcn.name ();
 
     m_os << (fcn_name.empty () ? "(empty)" : fcn_name) << ' ';
@@ -381,31 +354,9 @@
     tree_parameter_list *param_list = fcn.parameter_list ();
 
     if (param_list)
-      {
-        bool takes_varargs = fcn.takes_varargs ();
-
-        int len = param_list->length ();
-
-        if (len > 0 || takes_varargs)
-          {
-            m_os << '(';
-            m_nesting.push ('(');
-          }
-
-        param_list->accept (*this);
+      param_list->accept (*this);
 
-        if (len > 0 || takes_varargs)
-          {
-            m_nesting.pop ();
-            m_os << ')';
-            newline ();
-          }
-      }
-    else
-      {
-        m_os << "()";
-        newline ();
-      }
+    newline ();
   }
 
   void
@@ -737,6 +688,26 @@
   void
   tree_print_code::visit_parameter_list (tree_parameter_list& lst)
   {
+    bool is_input_list = lst.is_input_list ();
+
+    if (is_input_list)
+      {
+        m_os << '(';
+        m_nesting.push ('(');
+      }
+    else
+      {
+        int len = lst.length ();
+        if (lst.takes_varargs ())
+          len++;
+
+        if (len != 1)
+          {
+            m_os << '[';
+            m_nesting.push ('[');
+          }
+      }
+
     auto p = lst.begin ();
 
     while (p != lst.end ())
@@ -753,7 +724,25 @@
       }
 
     if (lst.takes_varargs ())
-      m_os << "varargin";
+      m_os << lst.varargs_symbol_name ();
+
+    if (is_input_list)
+      {
+        m_nesting.pop ();
+        m_os << ')';
+      }
+    else
+      {
+        int len = lst.length ();
+        if (lst.takes_varargs ())
+          len++;
+
+        if (len != 1)
+          {
+            m_nesting.pop ();
+            m_os << ']';
+          }
+      }
   }
 
   void