# HG changeset patch
# User Piotr Held <pjheld@gmail.com>
# Date 1502813064 25200
#      Tue Aug 15 09:04:24 2017 -0700
# Node ID c25a301053344262988ce643e6b37daa74949ab9
# Parent  e19a1c7a37697a65fb610d33d7a4be5c419f3bd7
Fixed parse error for abstract methods (Bug #51377) and attribute validation

* lex.h, lex.ll, oct-parse.in.yy: added looking_at_block_type property to keep track of what block is being parsed, looking_at_abstract_methods_block flag to allow parsing of empty methods outside of @-folders (Bug #51377).
* parse.h, oct-parse.in.yy (base_parser:: make_attribute): that does quite permissive attribute validation with the intent of finding simple user errors. It also marks if a methods block is abstract.
* ov-classdef.cc (compute_attribute_value): modified to support the tree_classdef_attribute::neg property.

* test/module.mk, bug-51377/module.mk, abs_bug51377.m, bug-51377.tst, child_bug51377.m: test files for this bug.

* +containers/Map.m: fixed improper syntax for property map. Currently SetAccess and GetAccess for this property are public (due to bad syntax).

diff -r e19a1c7a3769 -r c25a30105334 libinterp/octave-value/ov-classdef.cc
--- a/libinterp/octave-value/ov-classdef.cc	Fri Aug 11 16:21:56 2017 -0700
+++ b/libinterp/octave-value/ov-classdef.cc	Tue Aug 15 09:04:24 2017 -0700
@@ -2514,7 +2514,7 @@
       return tw.evaluate (expr);
     }
   else
-    return octave_value (true);
+    return octave_value (t->negate () == false);
 }
 
 template <typename T>
diff -r e19a1c7a3769 -r c25a30105334 libinterp/parse-tree/lex.h
--- a/libinterp/parse-tree/lex.h	Fri Aug 11 16:21:56 2017 -0700
+++ b/libinterp/parse-tree/lex.h	Tue Aug 15 09:04:24 2017 -0700
@@ -261,7 +261,10 @@
 
     lexical_feedback (void)
       : end_of_input (false), at_beginning_of_statement (true),
-        looking_at_anon_fcn_args (false), looking_at_return_list (false),
+        looking_at_anon_fcn_args (false),
+        looking_at_abstract_methods_block (false),
+        looking_at_block_type (block_type::none),
+        looking_at_return_list (false),
         looking_at_parameter_list (false), looking_at_decl_list (false),
         looking_at_initializer_expression (false),
         looking_at_matrix_or_assign_lhs (false),
@@ -329,6 +332,15 @@
     // true means we are parsing an anonymous function argument list.
     bool looking_at_anon_fcn_args;
 
+    // true means we are parsing a classdef methods, properties or events block
+    // with abstract attribute = true
+    bool looking_at_abstract_methods_block;
+    
+    // desrcibes the type of block we are currently looking at
+    enum block_type { none, classdef_block, enum_block, events_block,
+                      methods_block, properties_block };
+    block_type looking_at_block_type;
+
     // true means we're parsing the return list for a function.
     bool looking_at_return_list;
 
diff -r e19a1c7a3769 -r c25a30105334 libinterp/parse-tree/lex.ll
--- a/libinterp/parse-tree/lex.ll	Fri Aug 11 16:21:56 2017 -0700
+++ b/libinterp/parse-tree/lex.ll	Tue Aug 15 09:04:24 2017 -0700
@@ -2145,6 +2145,8 @@
     end_of_input = false;
     at_beginning_of_statement = true;
     looking_at_anon_fcn_args = false;
+    looking_at_abstract_methods_block = false;
+    looking_at_block_type = block_type::none;
     looking_at_return_list = false;
     looking_at_parameter_list = false;
     looking_at_decl_list = false;
@@ -2696,17 +2698,55 @@
             break;
 
           case enumeration_kw:
-          case events_kw:
-          case methods_kw:
-          case properties_kw:
-            // 'properties', 'methods' and 'events' are keywords for
-            // classdef blocks.
+            // 'enumeration' is keywords for classdef blocks
+            // only outside the methods block
             if (! parsing_classdef)
               {
                 at_beginning_of_statement = previous_at_bos;
                 return 0;
               }
-            // fall through ...
+
+            decrement_promptflag ();
+            looking_at_block_type = block_type::enum_block;
+            break;
+
+          case events_kw:
+            // 'events' is keywords for classdef blocks
+            // only outside the methods block
+            if (! parsing_classdef)
+              {
+                at_beginning_of_statement = previous_at_bos;
+                return 0;
+              }
+
+            decrement_promptflag ();
+            looking_at_block_type = block_type::events_block;
+            break;
+          case methods_kw:
+            // 'methods' is keywords for classdef blocks
+            // only outside the methods block
+            if (! parsing_classdef)
+              {
+                at_beginning_of_statement = previous_at_bos;
+                return 0;
+              }
+
+            decrement_promptflag ();
+            looking_at_block_type = block_type::methods_block;
+            break;
+
+          case properties_kw:
+            // 'properties' is keywords for classdef blocks
+            // only outside the methods block
+            if (! parsing_classdef)
+              {
+                at_beginning_of_statement = previous_at_bos;
+                return 0;
+              }
+
+            decrement_promptflag ();
+            looking_at_block_type = block_type::properties_block;
+            break;
 
           case classdef_kw:
             // 'classdef' is always a keyword.
@@ -2717,6 +2757,7 @@
                 reading_classdef_file = true;
                 reading_script_file = false;
               }
+            looking_at_block_type = block_type::classdef_block;
             break;
 
           case function_kw:
diff -r e19a1c7a3769 -r c25a30105334 libinterp/parse-tree/oct-parse.in.yy
--- a/libinterp/parse-tree/oct-parse.in.yy	Fri Aug 11 16:21:56 2017 -0700
+++ b/libinterp/parse-tree/oct-parse.in.yy	Tue Aug 15 09:04:24 2017 -0700
@@ -1662,19 +1662,26 @@
                 ;
 
 attr            : identifier
-                  { $$ = new octave::tree_classdef_attribute ($1); }
+                  {
+                    if (! ($$ = parser.make_attribute ($1, 0)))
+
+                      YYABORT;
+                  }
                 | identifier '=' decl_param_init expression
                   {
                     YYUSE ($2);
 
-                    lexer.looking_at_initializer_expression = false;
-                    $$ = new octave::tree_classdef_attribute ($1, $4);
+                    if (! ($$ = parser.make_attribute ($1, $4)))
+
+                      YYABORT;
                   }
                 | EXPR_NOT identifier
                   {
                     YYUSE ($1);
 
-                    $$ = new octave::tree_classdef_attribute ($2, false);
+                    if (! ($$ = parser.make_attribute ($2, 0, false)))
+
+                      YYABORT;
                   }
                 ;
 
@@ -3551,6 +3558,8 @@
   {
     tree_classdef *retval = nullptr;
 
+    m_lexer.looking_at_block_type = lexical_feedback::block_type::none;
+
     m_lexer.symtab_context.pop ();
 
     std::string cls_name = id->name ();
@@ -3601,6 +3610,266 @@
     return retval;
   }
 
+    // Build an attribute expression and check for valid attributes
+    // This is a partial implementation that is suppose to help developers
+    // avoid misspeling attribute names such as Asstract instead of Abstract.
+    // It is more permissive than not and does NOT check whether certain
+    // features are actually implemented in Octave.
+    tree_classdef_attribute *
+    base_parser::make_attribute (tree_identifier *id,
+                                 tree_expression *expr,
+                                 bool set_attrb)
+    {
+      bool accept_type_bool = false;
+      bool accept_type_enum = false;
+      bool accept_type_meta = false;
+
+      std::string name = id->name ();
+
+      // First check for recognized identifier names
+      switch (m_lexer.looking_at_block_type)
+        {
+        case lexical_feedback::block_type::classdef_block:
+          if (name == std::string ("Abstract")
+              || name == std::string ("ConstructOnLoad")
+              || name == std::string ("HandleCompatible")
+              || name == std::string ("Hidden")
+              || name == std::string ("Sealed"))
+            accept_type_bool = true;
+          else if (name == std::string ("AllowedSubclasses")
+                   || name == std::string ("InferiorClasses"))
+            accept_type_meta = true;
+          else
+            bison_error ("unrecognized classdef attribute: " + name);
+          break;
+
+        case lexical_feedback::block_type::events_block:
+          if (name == std::string ("Hidden"))
+            accept_type_bool = true;
+          else if (name == std::string ("ListenAccess")
+                   || name == std::string ("NotifyAccess"))
+            {
+              accept_type_enum = true;
+              accept_type_meta = true;
+            }
+          else
+            bison_error ("unrecognized event attribute: " + name);
+          break;
+
+        case lexical_feedback::block_type::methods_block:
+          if (name == std::string ("Abstract")
+              || name == std::string ("Hidden")
+              || name == std::string ("Sealed")
+              || name == std::string ("Static"))
+            accept_type_bool = true;
+          else if (name == std::string ("Access"))
+            {
+              accept_type_enum = true;
+              accept_type_meta = true;
+            }
+          else
+            bison_error ("unrecognized methods attribute: " + name);
+          break;
+
+        case lexical_feedback::block_type::properties_block:
+          if (name == std::string ("AbortSet")
+              || name == std::string ("Abstract")
+              || name == std::string ("Constant")
+              || name == std::string ("Dependent")
+              || name == std::string ("GetObservable")
+              || name == std::string ("Hidden")
+              || name == std::string ("NonCopyable")
+              || name == std::string ("SetObservable")
+              || name == std::string ("Transient"))
+            accept_type_bool = true;
+          else if (name == std::string ("GetAccess")
+                   || name == std::string ("SetAccess"))
+            accept_type_enum = true;
+          else if (name == std::string ("Access"))
+            {
+              accept_type_enum = true;
+              accept_type_meta = true;
+            }
+          else
+            bison_error ("unrecognized properties attribute: " + name);
+          break;
+
+          // For other situations when attributes aren't supported
+          case lexical_feedback::block_type::enum_block:
+          case lexical_feedback::block_type::none:
+            bison_error ("this context does not support attributes");
+            break;
+          // If the attribute is not in the list
+          default:
+            bison_error ("unrecognized attribute: " + name);
+        }
+
+
+      // Evaluate if attributes name makes sense and create output
+      tree_classdef_attribute *retval = nullptr;
+
+      // Now we'll check if we need to evaluate expr and if yes we'll evaluate it
+      // and return the appropriate tree_classdef_attribute object
+      if (expr)
+        {
+          // If it's an identifier it needs to be one of the following:
+          //
+          //    public, protected, private, immutable
+          //
+          // Although some attributes only support the first 3
+          if (expr->is_identifier ())
+            {
+              std::string val_nm = expr->name ();
+              // Check if identifier is a enum type
+              if (accept_type_enum 
+                  && (val_nm == std::string ("public")
+                      || val_nm == std::string ("protected")
+                      || val_nm == std::string ("private")
+                      || val_nm == std::string ("immutable")))
+                retval = new tree_classdef_attribute (id, expr);
+              // Check if identifier is a bool
+              else if (accept_type_bool
+                       && (val_nm == std::string ("false")
+                           || val_nm == std::string ("true")))
+                retval 
+                  = new tree_classdef_attribute (id, val_nm
+                                                     == std::string ("false"));
+
+              else
+                bison_error ("unrecognized value " + val_nm
+                             + " for attribute: " + name);
+            }
+          else
+            {
+
+              // FIXME: If someone assigns an attribute using a metaclass
+              // expression and the parent object tries to get the child 
+              // meta.class object e.g.:
+              //
+              //    methods (Access = ?childClass)
+              //
+              // Then if we try to evaluate '?childClass' we will go into
+              // an endless loop of trying to parse the child then trying
+              // to parse the parent and so on that causes stack overflow.
+              // Even a simple check on the stack to see if the current
+              // function is __meta_class_query__ will not work correctly
+              // and you will end up parsing childClass twice at the same
+              // time: once when simply parsing it, and later when generating
+              // a meta.class object for 'childClass' the value of property
+              // Access (of the parent class in this example)
+              // also needs to be determined.
+              // Since this is suppose to help avoid stupid errors
+              // we will avoid evaluating meta.class calls at this stage.
+              // We mainly want evaluate to evaluate simple true/false
+              // statements.
+
+              std::string input_line_text = m_lexer.current_input_line;
+
+              // We also will check if __make_class_meta__ is on the stack
+              // so that if the syntax changes we won't get stack overflow.
+              interpreter *interp = interpreter::the_interpreter ();
+              call_stack &stack = interp->get_call_stack ();
+              octave_function *caller = stack.current ();
+              std::string caller_nm = caller ? caller->name ()
+                                             : std::string("");
+
+              if (input_line_text.find('?') == std::string::npos
+                  && input_line_text.find("metaclass") == std::string::npos
+                  && input_line_text.find("meta.class") == std::string::npos
+                  && caller_nm != std::string ("__meta_class_query__"))
+                {
+                  try 
+                    {
+                      tree_evaluator& tw
+                        = __get_evaluator__ ("make_attribute");
+
+                      octave_value tmp = tw.evaluate (expr);
+
+                      if (tmp.numel () == 1 && tmp.isnumeric ()
+                          && accept_type_bool)
+                        {
+                          // The second argument negates the
+                          // attribute, so we need a double negative
+                          retval 
+                            = new tree_classdef_attribute (id,
+                                                           !tmp.bool_value ());
+                        }
+                      else if (tmp.is_string ()
+                               && accept_type_enum)
+                        {
+                          std::string val_nm = tmp.string_value ();
+                          if (val_nm == std::string ("public")
+                              || val_nm == std::string ("protected")
+                              || val_nm == std::string ("private")
+                              || val_nm == std::string ("immutable"))
+                            retval = new tree_classdef_attribute (id, expr);
+                          else
+                            bison_error ("unrecognized value " + val_nm
+                                         + " for attribute: " + name);
+                        }
+                      else
+                        bison_error ("unrecognized type " + tmp.type_name ()
+                                     + " for attribute: " + name);
+
+                    }
+                  catch (const execution_exception&)
+                    {
+                      interpreter::recover_from_exception ();
+                    }
+                }
+              // We cannot run evaluate and check if the attribute value
+              // is correct so we deffer it till compute_attribute_value ()
+              else if (accept_type_meta)
+                {
+                  retval 
+                    = new tree_classdef_attribute (id, expr);
+                }
+              else
+                bison_error ("unrecognized type for attribute: " + name);
+
+            }
+        }
+      // This is for the cases: identifier or NEG identifier
+      // If accept_type_bool is true that means the identifier
+      // was consider a valid identifier
+      else if (accept_type_bool)
+        {
+          // Since tree_classdef_attribute accepts negate as
+          // the parameter we need to double negate to get
+          // a positive
+          retval
+            = new tree_classdef_attribute (id, !set_attrb);
+        }
+
+      // Mark abstract methods block as such
+      // only if the parsing has been successful so far
+      if (retval)
+        {
+          // Applicable only for method block
+          if (accept_type_bool
+              && m_lexer.looking_at_block_type
+                 == lexical_feedback::block_type::methods_block)
+            {
+              // Check if it's the correct attribute and
+              // if we are actually turning it on
+              if (name == std::string ("Abstract")
+                  && retval->negate () == false)
+                m_lexer.looking_at_abstract_methods_block = true;
+            }
+        }
+      // Cleanup after error
+      else /*!retval*/
+        {
+          delete id;
+          delete expr;
+        }
+
+      return retval;
+
+    }
+
+
+
   tree_classdef_properties_block *
   base_parser::make_classdef_properties_block (token *tok_val,
                                                tree_classdef_attribute_list *a,
@@ -3610,6 +3879,9 @@
   {
     tree_classdef_properties_block *retval = nullptr;
 
+    m_lexer.looking_at_block_type
+      = lexical_feedback::block_type::classdef_block;
+
     if (end_token_ok (end_tok, token::properties_end))
       {
         comment_list *tc = m_lexer.comment_buf.get_comment ();
@@ -3642,6 +3914,10 @@
   {
     tree_classdef_methods_block *retval = nullptr;
 
+    m_lexer.looking_at_abstract_methods_block = false;
+    m_lexer.looking_at_block_type
+      = lexical_feedback::block_type::classdef_block;
+
     if (end_token_ok (end_tok, token::methods_end))
       {
         comment_list *tc = m_lexer.comment_buf.get_comment ();
@@ -3674,6 +3950,9 @@
   {
     tree_classdef_events_block *retval = nullptr;
 
+    m_lexer.looking_at_block_type 
+      = lexical_feedback::block_type::classdef_block;
+
     if (end_token_ok (end_tok, token::events_end))
       {
         comment_list *tc = m_lexer.comment_buf.get_comment ();
@@ -3706,6 +3985,9 @@
   {
     tree_classdef_enum_block *retval = nullptr;
 
+    m_lexer.looking_at_block_type
+      = lexical_feedback::block_type::classdef_block;
+
     if (end_token_ok (end_tok, token::enumeration_end))
       {
         comment_list *tc = m_lexer.comment_buf.get_comment ();
@@ -3737,8 +4019,11 @@
 
     // External methods are only allowed within @-folders. In this case,
     // m_curr_class_name will be non-empty.
-
-    if (! m_curr_class_name.empty ())
+    // We also need to create a dummy function for Abstract methods 
+    // which will return empty on m_curr_class_name
+
+    if (! m_curr_class_name.empty ()
+        || m_lexer.looking_at_abstract_methods_block)
       {
 
         std::string mname = id->name ();
diff -r e19a1c7a3769 -r c25a30105334 libinterp/parse-tree/parse.h
--- a/libinterp/parse-tree/parse.h	Fri Aug 11 16:21:56 2017 -0700
+++ b/libinterp/parse-tree/parse.h	Tue Aug 15 09:04:24 2017 -0700
@@ -317,6 +317,11 @@
                    tree_classdef_body *body, token *end_tok,
                    comment_list *lc);
 
+    // Build an attribute expression
+    tree_classdef_attribute *
+    make_attribute (tree_identifier *id, tree_expression *expr,
+                    bool set_attrb = true);
+
     tree_classdef_properties_block *
     make_classdef_properties_block (token *tok_val,
                                     tree_classdef_attribute_list *a,
diff -r e19a1c7a3769 -r c25a30105334 scripts/+containers/Map.m
--- a/scripts/+containers/Map.m	Fri Aug 11 16:21:56 2017 -0700
+++ b/scripts/+containers/Map.m	Tue Aug 15 09:04:24 2017 -0700
@@ -127,7 +127,7 @@
     Count = 0;
   endproperties
 
-  properties (private)
+  properties (Access = private)
     map = struct ();
 
     numeric_keys = false;
diff -r e19a1c7a3769 -r c25a30105334 test/bug-51377/abs_bug51377.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-51377/abs_bug51377.m	Tue Aug 15 09:04:24 2017 -0700
@@ -0,0 +1,8 @@
+classdef abs_bug51377 < handle
+  methods (Abstract)
+    foo (self)
+  endmethods
+  methods (Abstract = true)
+    bar (self)
+  endmethods
+endclassdef
diff -r e19a1c7a3769 -r c25a30105334 test/bug-51377/bug-51377.tst
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-51377/bug-51377.tst	Tue Aug 15 09:04:24 2017 -0700
@@ -0,0 +1,22 @@
+## 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 ();
diff -r e19a1c7a3769 -r c25a30105334 test/bug-51377/child_bug51377.m
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-51377/child_bug51377.m	Tue Aug 15 09:04:24 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 e19a1c7a3769 -r c25a30105334 test/bug-51377/module.mk
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/bug-51377/module.mk	Tue Aug 15 09:04:24 2017 -0700
@@ -0,0 +1,6 @@
+bug_51377_TEST_FILES = \
+  %reldir%/bug-51377.tst \
+  %reldir%/abs_bug51377.m \
+  %reldir%/child_bug51377.m
+
+TEST_FILES += $(bug_51377_TEST_FILES)
diff -r e19a1c7a3769 -r c25a30105334 test/module.mk
--- a/test/module.mk	Fri Aug 11 16:21:56 2017 -0700
+++ b/test/module.mk	Tue Aug 15 09:04:24 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
