# HG changeset patch
# User Piotr Held <pjheld@gmail.com>
# Date 1503692221 25200
#      Fri Aug 25 13:17:01 2017 -0700
# Node ID 3f55a05f40361b5a87c7048d2bc1966d420d11fd
# Parent  fd6b1339689f7a72ce4860b487aa32ff4562558e
fix parse error for abstract methods outside @-folder (Bug #51377)

* lex.h, lex.ll, oct-parse.in.yy: If a function signature (without the function definition) exists in methods block outside @-folder the parser no longer gives error, instead, the newly added lexical_feedback::methods_block_has_function_signatures is set to true and the error is dealt with during class creation.
* ov-classdef.cc, pt-classdef.h (tree_classdef_methods_block: new private property m_has_function_signatures and getter has_function_signatures ().
* ov-classdef.cc: calculate value of attribute and check if methods block is abstract. If it is not and the block has_function_signatures () then throw error disallowing external methods outside @-folders.
* ov-classdef.cc, pt-classdef.h (tree_classdef_attribute): refactored property 'neg' to 'bool_value' so that passing true to constructor sets attribute value to true and not to false.
* ov-classdef.cc (compute_attribute_value): return ov true or false depending on weather the attribute is set to true or false.
* test/module.mk, test/bug-51377* : test files for the problem.

diff -r fd6b1339689f -r 3f55a05f4036 libinterp/octave-value/ov-classdef.cc
--- a/libinterp/octave-value/ov-classdef.cc	Thu Aug 24 14:51:31 2017 -0700
+++ b/libinterp/octave-value/ov-classdef.cc	Fri Aug 25 13:17:01 2017 -0700
@@ -2514,7 +2514,7 @@
       return tw.evaluate (expr);
     }
   else
-    return octave_value (true);
+    return octave_value (t->get_bool_value ());
 }
 
 template <typename T>
@@ -2628,9 +2628,10 @@
 #endif
 
           // Method attributes
-
+          bool is_abstract_methods_block = false;
           if (mb_p->attribute_list ())
             {
+
               for (auto& attr_p : *mb_p->attribute_list ())
                 {
                   std::string aname = attr_p->ident ()->name ();
@@ -2641,10 +2642,21 @@
                             << attribute_value_to_string (attr_p, avalue)
                             << std::endl;
 #endif
-
+                  // FIXME: Add checker for proper attribute/value combinations
                   amap[aname] = avalue;
+
+                  // This cannot be determined earlier since avalue
+                  // was not known earlier.
+                  if (aname == std::string ("Abstract")
+                      && avalue.bool_value ())
+                    is_abstract_methods_block = true;
                 }
             }
+          if (mb_p->has_function_signatures ()
+              && is_abstract_methods_block == false)
+            {
+              error ("external methods are only allowed in @-folders");
+            }
 
           // Methods
 
diff -r fd6b1339689f -r 3f55a05f4036 libinterp/parse-tree/lex.h
--- a/libinterp/parse-tree/lex.h	Thu Aug 24 14:51:31 2017 -0700
+++ b/libinterp/parse-tree/lex.h	Fri Aug 25 13:17:01 2017 -0700
@@ -268,6 +268,7 @@
         looking_for_object_index (false),
         looking_at_indirect_ref (false), parsing_class_method (false),
         parsing_classdef (false), maybe_classdef_get_set_method (false),
+        methods_block_has_function_signatures (false),
         parsing_classdef_get_method (false),
         parsing_classdef_set_method (false),
         quote_is_transpose (false), force_script (false),
@@ -365,6 +366,10 @@
     // for example, "get.propertyname" is recognized as a function name.
     bool maybe_classdef_get_set_method;
 
+    // true means the current methods block contains function signatures
+    // outside an @-folder instead of full function definitions
+    bool methods_block_has_function_signatures;
+
     // TRUE means we are parsing a classdef get.method.
     bool parsing_classdef_get_method;
 
diff -r fd6b1339689f -r 3f55a05f4036 libinterp/parse-tree/lex.ll
--- a/libinterp/parse-tree/lex.ll	Thu Aug 24 14:51:31 2017 -0700
+++ b/libinterp/parse-tree/lex.ll	Fri Aug 25 13:17:01 2017 -0700
@@ -2155,6 +2155,7 @@
     parsing_class_method = false;
     parsing_classdef = false;
     maybe_classdef_get_set_method = false;
+    methods_block_has_function_signatures = false;
     parsing_classdef_get_method = false;
     parsing_classdef_set_method = false;
     quote_is_transpose = false;
diff -r fd6b1339689f -r 3f55a05f4036 libinterp/parse-tree/oct-parse.in.yy
--- a/libinterp/parse-tree/oct-parse.in.yy	Thu Aug 24 14:51:31 2017 -0700
+++ b/libinterp/parse-tree/oct-parse.in.yy	Fri Aug 25 13:17:01 2017 -0700
@@ -3649,10 +3649,12 @@
         int l = tok_val->line ();
         int c = tok_val->column ();
 
+        bool fs = m_lexer.methods_block_has_function_signatures;
+
         if (! mlist)
           mlist = new tree_classdef_methods_list ();
 
-        retval = new tree_classdef_methods_block (a, mlist, lc, tc, l, c);
+        retval = new tree_classdef_methods_block (fs, a, mlist, lc, tc, l, c);
       }
     else
       {
@@ -3662,6 +3664,8 @@
         end_token_error (end_tok, token::methods_end);
       }
 
+    m_lexer.methods_block_has_function_signatures = false;
+
     return retval;
   }
 
@@ -3735,42 +3739,44 @@
   {
     octave_user_function* retval = nullptr;
 
-    // External methods are only allowed within @-folders. In this case,
-    // m_curr_class_name will be non-empty.
-
-    if (! m_curr_class_name.empty ())
+    // External methods are only allowed within @-folders.
+    // If m_curr_class_name is empty that means that this block
+    // is outside @-folder. Since this method is called we are parsing
+    // only a function signature instead of a full function.
+    // Possibly it is an abstract methods block, however, this needs to be
+    // checked when evaluating attribute values at class creation.
+    if (m_curr_class_name.empty ())
       {
-
-        std::string mname = id->name ();
-
-        // Methods that cannot be declared outside the classdef file:
-        // - methods with '.' character (e.g. property accessors)
-        // - class constructor
-        // - `delete'
-
-        if (mname.find_first_of (".") == std::string::npos
-            && mname != "delete"
-            && mname != m_curr_class_name)
-          {
-            // Create a dummy function that is used until the real method
-            // is loaded.
-
-            retval = new octave_user_function (nullptr, pl);
-
-            retval->stash_function_name (mname);
-
-            int l = id->line ();
-            int c = id->column ();
-
-            retval->stash_fcn_location (l, c);
-          }
-        else
-          bison_error ("invalid external method declaration, an external "
-                       "method cannot be the class constructor, `delete' "
-                       "or have a dot (.) character in its name");
+        m_lexer.methods_block_has_function_signatures = true;
+      }
+
+    std::string mname = id->name ();
+
+    // Methods that cannot be declared outside the classdef file:
+    // - methods with '.' character (e.g. property accessors)
+    // - class constructor
+    // - `delete'
+
+    if (mname.find_first_of (".") == std::string::npos
+        && mname != "delete"
+        && mname != m_curr_class_name)
+      {
+        // Create a dummy function that is used until the real method
+        // is loaded.
+
+        retval = new octave_user_function (nullptr, pl);
+
+        retval->stash_function_name (mname);
+
+        int l = id->line ();
+        int c = id->column ();
+
+        retval->stash_fcn_location (l, c);
       }
     else
-      bison_error ("external methods are only allowed in @-folders");
+      bison_error ("invalid external method declaration, an external "
+                   "method cannot be the class constructor, `delete' "
+                   "or have a dot (.) character in its name");
 
     if (! retval)
       delete id;
diff -r fd6b1339689f -r 3f55a05f4036 libinterp/parse-tree/pt-classdef.h
--- a/libinterp/parse-tree/pt-classdef.h	Thu Aug 24 14:51:31 2017 -0700
+++ b/libinterp/parse-tree/pt-classdef.h	Fri Aug 25 13:17:01 2017 -0700
@@ -47,10 +47,10 @@
 
     tree_classdef_attribute (tree_identifier *i = nullptr,
                              tree_expression *e = nullptr)
-      : id (i), expr (e), neg (false) { }
+      : id (i), expr (e), bool_value (true) { }
 
     tree_classdef_attribute (tree_identifier *i, bool b)
-      : id (i), expr (nullptr), neg (b) { }
+      : id (i), expr (nullptr), bool_value (b) { }
 
     // No copying!
 
@@ -68,7 +68,7 @@
 
     tree_expression * expression (void) { return expr; }
 
-    bool negate (void) { return neg; }
+    bool get_bool_value (void) { return bool_value; }
 
     void accept (tree_walker& tw)
     {
@@ -79,7 +79,7 @@
 
     tree_identifier *id;
     tree_expression *expr;
-    bool neg;
+    bool bool_value;
   };
 
   class tree_classdef_attribute_list : public base_list<tree_classdef_attribute *>
@@ -331,11 +331,13 @@
   {
   public:
 
-    tree_classdef_methods_block (tree_classdef_attribute_list *a,
+    tree_classdef_methods_block (bool has_fs,
+                                 tree_classdef_attribute_list *a,
                                  tree_classdef_methods_list *mlist,
                                  comment_list *lc,
                                  comment_list *tc, int l = -1, int c = -1)
-      : tree_classdef_element<octave_value> (a, mlist, lc, tc, l, c) { }
+      : tree_classdef_element<octave_value> (a, mlist, lc, tc, l, c),
+        m_has_function_signatures (has_fs) { }
 
     // No copying!
 
@@ -346,10 +348,16 @@
 
     ~tree_classdef_methods_block (void) = default;
 
+    bool has_function_signatures () { return m_has_function_signatures; }
+
     void accept (tree_walker& tw)
     {
       tw.visit_classdef_methods_block (*this);
     }
+
+  private:
+    // True if methods block contains function signatures outside @-folder
+    bool m_has_function_signatures;
   };
 
   class tree_classdef_event
diff -r fd6b1339689f -r 3f55a05f4036 test/bug-51377/@inside_bug51377/foo.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-51377/@inside_bug51377/foo.m	Fri Aug 25 13:17:01 2017 -0700
@@ -0,0 +1,3 @@
+function foo (self)
+  1;
+end
diff -r fd6b1339689f -r 3f55a05f4036 test/bug-51377/@inside_bug51377/inside_bug51377.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-51377/@inside_bug51377/inside_bug51377.m	Fri Aug 25 13:17:01 2017 -0700
@@ -0,0 +1,5 @@
+classdef inside_bug51377 < abs_bug51377
+  methods
+    foo (self)
+  endmethods
+endclassdef
diff -r fd6b1339689f -r 3f55a05f4036 test/bug-51377/abs_bug51377.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-51377/abs_bug51377.m	Fri Aug 25 13:17:01 2017 -0700
@@ -0,0 +1,8 @@
+classdef abs_bug51377 < handle
+  methods (Abstract)
+    foo (self)
+  endmethods
+  methods (Abstract = true)
+    bar (self)
+  endmethods
+endclassdef
diff -r fd6b1339689f -r 3f55a05f4036 test/bug-51377/bug-51377.tst
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-51377/bug-51377.tst	Fri Aug 25 13:17:01 2017 -0700
@@ -0,0 +1,28 @@
+## Copyright (C) 2017 Piotr Held
+##
+## This file is part of Octave.
+##
+## Octave is free software; you can redistribute it and/or modify it
+## under the terms of the GNU General Public License as published by
+## the Free Software Foundation; either version 3 of the License, or
+## (at your option) any later version.
+##
+## Octave is distributed in the hope that it will be useful, but
+## WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+## GNU General Public License for more details.
+##
+## You should have received a copy of the GNU General Public License
+## along with Octave; see the file COPYING.  If not, see
+## <http://www.gnu.org/licenses/>.
+
+%!test
+%! a = child_bug51377;
+%! a.foo ();
+%! a.bar ();
+
+%% Check if bugfix did not break previous inside-outside @-folder checks.
+%!test
+%! fail ('outside_bug51377');
+%! b = inside_bug51377;
+%! b.foo (); 
diff -r fd6b1339689f -r 3f55a05f4036 test/bug-51377/child_bug51377.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-51377/child_bug51377.m	Fri Aug 25 13:17:01 2017 -0700
@@ -0,0 +1,10 @@
+classdef child_bug51377 < abs_bug51377
+  methods
+    function foo (self)
+      1;
+    endfunction
+    function bar (self)
+      1;
+    endfunction
+  endmethods
+endclassdef
diff -r fd6b1339689f -r 3f55a05f4036 test/bug-51377/module.mk
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-51377/module.mk	Fri Aug 25 13:17:01 2017 -0700
@@ -0,0 +1,9 @@
+bug_51377_TEST_FILES = \
+  %reldir%/bug-51377.tst \
+  %reldir%/abs_bug51377.m \
+  %reldir%/child_bug51377.m \
+  %reldir%/outside_bug51377.m \
+  %reldir%/@inside_bug51377/@inside_bug51377.m \
+  %reldir%/@outside_bug51377/foo.m
+
+TEST_FILES += $(bug_51377_TEST_FILES)
diff -r fd6b1339689f -r 3f55a05f4036 test/bug-51377/outside_bug51377.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-51377/outside_bug51377.m	Fri Aug 25 13:17:01 2017 -0700
@@ -0,0 +1,5 @@
+classdef outside_bug51377 < abs_bug51377
+  methods
+    foo (self)
+  endmethods
+endclassdef
diff -r fd6b1339689f -r 3f55a05f4036 test/module.mk
--- a/test/module.mk	Thu Aug 24 14:51:31 2017 -0700
+++ b/test/module.mk	Fri Aug 25 13:17:01 2017 -0700
@@ -57,6 +57,7 @@
 include %reldir%/bug-50035/module.mk
 include %reldir%/bug-50716/module.mk
 include %reldir%/bug-51192/module.mk
+include %reldir%/bug-51377/module.mk
 include %reldir%/bug-51532/module.mk
 include %reldir%/bug-51534/module.mk
 include %reldir%/bug-51599/module.mk
