Mon 18 Dec 2017 06:18:58 PM UTC, comment #10:
The old patch is no longer compatible with tip. Therefore, I created an updated version against a51a92186a1f421a35ed2bdf63a39.
It's name is patch_ver3_against_a51a92186a1.diff
(file #42674)
|
Tue 05 Sep 2017 11:19:12 PM UTC, comment #9:
I am very sorry however in comment #6 I attached the wrong patch... I am sorry for the confusion.
I intended to attach the patch that I am attaching now.
The comments I wrote actually refer to the code in this patch.
(file #41739)
|
Wed 30 Aug 2017 05:52:47 PM UTC, comment #8:
I started thinking if my solution does not cause a memory leak, because I raise an error inside make_meta_class. Sure enough it seems like it did cause a memory leak. However, there was another error raised in cdef_class::make_meta_class, so the memory leak existed prior to this new cset. That memory leak happened when creating 'sl_child' in the code below:
So I made a patch that should solve the problem of raising errors inside cdef_class::make_meta_class and I am attachin it here. This patch should work with the current tip (23973:a315ac23dc6c).
(file #41701)
|
Fri 25 Aug 2017 11:27:25 PM UTC, comment #7:
I am sorry, a typo got into my message:
This patch does not throw errors if you attemp to create an instance of an abstract class.
|
Fri 25 Aug 2017 11:15:01 PM UTC, comment #6:
I wrote a new patch in the file simple_patch_bug51377.diff
This is a much simpler patch that only deals with this bug. It does not solve the attribute validation problem, though I did add a FIXME about it in the classdef creation code. I would be willing to write the attribute valiadion inside ov-classdef.cc, however, that is not necessary for this problem to be solved.
This patch does throw errors if you attempt to can create an instance of an abstract class. That is an attribute validation problem and I wanted to keep this patch as simple as possible.
Apart from fixing the bug, the only additional thing I did was refactoring tree_classdef_attribute. Previously if an attribute was negated (e.g. 'methods (~Abstract)') tree_classdef_attribute was designed to hold that information as true (as in tree_classdef_attribute::negate = true). It seems more intuitive to hold the value as false, since we are actually setting the value of the attribute to false (in the refactored version: tree_classdef_attribute::bool_value = false).
I also added a bunch of tests.
(file #41663)
|
Thu 17 Aug 2017 11:46:56 PM UTC, comment #5:
I wanted to write down somewhere our conversation from IRC and point to the problems with my cset and how to solve parsing problem. For Octave it is important to know if a method block has attribute Abstract. This is quite easy if the declaration looks like this:
But that is not the only allowed syntax. Attributes can be called using 'Attrib_Name = value'. This 'value' can be be anything from 'true', 'false', 'private' to functions and metaclasses. Currently the interpreter does no input validation on attributes at all and assigns their value during class creation (using the information stored in a parse-tree). I have not done this yet, but I was gonna propose to evaluate the value of an Attribute at parse-time and assign it there, so that the parse tree holds that value. Otherwise, if we evaluate the attribute value at parse-time to check what it is, we will have to evaluate it again during class creation. What do you think of this proposal?
Also does anyone have an idea how to solve the recursive attribute lookup question? (Right now I just simply avoid evaluating '?' and 'metaclass' calls when parsing attributes). My idea was to look at the call stack and see if the caller is a _make_class_meta_ call, but I think that isn't all that great.
To give examples of what I'm talking about:
If we evaluate the value of Access at parse-time we create a recursive parse lookup when invoking the constructor of either the parent or the chld (when we create the child we need to parse it->to parse the child we need to parse the parent->when parsing the parent we look at attribute Access and then we try to get it's value->to get it's value we need to parse the child-> and so on). Any suggestions?
|
Thu 17 Aug 2017 12:21:09 AM UTC, comment #4:
I did some more testing and found out that:
1. 'private' is supported by Matlab not only private (so string types).
2. containers.Map had a syntax error which I also fixed in this cset (I say syntax error because property 'map' was not private even though it was marked as such by the person who wrote this classdef).
3. Logical attributes accept not only logical but basically numerical scalars.
(file #41551)
|
Wed 16 Aug 2017 10:44:11 PM UTC, comment #3:
So I have a cset that solves this problem and provides attribute validation. As I explain in the comments the intent is for it to be more permissive than not. It's weakest link is assigning attributes a metaclass values. The problem arises in recursive metaclass calls. However, this functionality isn't perfect anyway in Octave.
I would like to create a number of tests for various cases present here, however I do not know where to do that. It seems that the bug test space is not a proper place for it. Where should I put these tests?
(file #41550)
|
Tue 15 Aug 2017 05:01:22 PM UTC, comment #2:
After some discussion on IRC, we decided to do better validation of classdef attributes in the parser as a first step toward fixing this problem.
|
Tue 15 Aug 2017 04:14:08 PM UTC, comment #1:
The way I solved this problem is I created a lexical_feedback flag called looking_at_abstract_attribute_block. This is true when an attr (in oct-parse.in.yy) with name "Abstract" appears (or an assignment "Abstract = true"). If the flag is true then base_parser::start_classdef_external_method creates a dummy function for the abstract method and everything parses correctly (just as it would if the abstract class was in an @-folder).
I will open a separate bug that concerns the fact that you can instantiate abstract classes (even now if placed in @-folders an abstract class can be instantiated). I will attempt to solve that problem if the solution to this problem is acceptable.
I also added tests for this particular problem.
(file #41527)
|
Mon 03 Jul 2017 11:27:33 PM UTC, original submission:
GNU Octave shows parse error for abstract methods in abstract classes defined with "classdef".
For example, we have an abstract class:
And a concrete class:
Running command "ConcreteIncrementClass.Increment(1)" results in error message (path removed):
In Matlab it gives the answer (2).
Without "Static" keyword the same error message is shown while running the constructor of concrete class (Matlab shows no error here as well).
|