bugGNU Octave - Bugs: bug #65037, constructor argument getting lost

 
 

bug #65037: constructor argument getting lost

Submitter:  Ray Zimmerman <rdzman>
Submitted:  Fri 15 Dec 2023 09:37:15 PM UTC
   
 
Category:  Classdef Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  rdzman Open/Closed:  * Closed
Release:  * 8.4.0 Operating System:  * Any
Fixed Release:  9.1.0 (current stable) Planned Release:  9.1.0 (current stable)
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 30 Jan 2024 11:19:01 PM UTC, comment #6: 

Verified patch.  Marking as Fixed and closing report.

Rik <rik5>
Group administrator
Fri 26 Jan 2024 09:24:18 PM UTC, comment #5: 

I pushed the following change to stable and merged with default.

http://hg.savannah.gnu.org/hgweb/octave/rev/833f3d9bdf1a

There is a new test and the new iteration of the patch doesn't require new functions, so there shouldn't be any visibility issues with the change.

John W. Eaton <jwe>
Group administrator
Fri 26 Jan 2024 05:14:43 PM UTC, comment #4: 

I also find that part of the code pretty confusing.

If I understand your changes correctly, it let's the fcn_info class know that superclass names aren't actually "proper" method names of a classdef subclass. I.e., the test code from comment #0 didn't actually mean to call `b3 = s.mybaseclass();`.

I'm struggling to understand the `else`-part (search parent classes). Is `name` the name of the class of `s` in that example?

I agree that a method that would do what you describe in the FIXME note would make that a lot easier to understand. But that can also happen later.

As far as I understand them, the changes look good to me. I think it is ok to fix this on stable.
However, the code didn't link with enabled visibility attributes on Windows unless I added an API flag to the declaration of the new function that is defined outside the header. See the attached updated patch.

A BIST for this would be nice. 👍


(file #55617)

Markus Mützel <mmuetzel>
Group administrator
Fri 12 Jan 2024 09:38:22 PM UTC, comment #3: 

I don't think it is the best fix, but the attached change appears to avoid the problem for me.  While working on this problem I was once again reminded of how confusing the classdef code in Octave is (at least to me).  For this particular case, we might want to combine the search for methods and constructors instead of having to ignore constructors that are mis-identified as methods only to have to search for them again as constructors!  Unfortunately, any other fix that I looked at meant a much larger change that I'm just not willing to take on at the moment.

Before I push this fix I'd like to add some tests and also decide whether it should go on stable for the upcoming release or on default.



(file #55557)

John W. Eaton <jwe>
Group administrator
Mon 18 Dec 2023 04:21:43 PM UTC, comment #2: 

Thanks, but unfortunately, in my real-world context that isn't really an option since the constructor needs to accept an arbitrary number of arguments of arbitrary type. That is, both cell arrays and objects are valid arguments, but are treaded differently.

But I think I found another design-level work-around that avoids problem entirely for the time being. Having this bug fixed would be much cleaner though ;-)

Thanks,

Ray Zimmerman <rdzman>
Fri 15 Dec 2023 11:53:36 PM UTC, comment #1: 

You could wrap the arg in a cell to fool the function lookup as a work around.


b3 = mybaseclass({s});


Petter <petter>
Fri 15 Dec 2023 09:37:15 PM UTC, original submission:  

I encountered a strange bug where an argument passed to a constructor of a classdef object is getting lost.

I have been able to reproduce it on multiple OSs in versions from 5.2.0 to 8.4.0 with the following minimal working example.

Given these two classes (also attached) ...

classdef mybaseclass
    methods
        function obj = mybaseclass(varargin)
            fprintf('number of constructor arguments = %d\n', nargin);
        end
    end
end

classdef mysubclass < mybaseclass
    methods
        function obj = mysubclass(varargin)
            obj@mybaseclass(varargin{:});
        end
    end
end


This code ...

b = mybaseclass('baseclass_arg');
s = mysubclass('subclass_arg');
b2 = mybaseclass(b);
b3 = mybaseclass(s);


... outputs ...

number of constructor arguments = 1
number of constructor arguments = 1
number of constructor arguments = 1
number of constructor arguments = 0


The last case should also see 1 argument (as MATLAB does, btw), but it's missing.

From my testing, it appears that if any of the arguments to a constructor is an object of a subclass, it will eat the first argument passed in (even if the subclassed object is not the first argument).

Any workarounds appreciated ... I need to be able to pass that subclassed object to the constructor of a parent class object.

Ray Zimmerman <rdzman>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #55617:  bug-65037-v2.patch added by mmuetzel (4KiB - text/plain)
file #55557:  bug-65037.txt added by jwe (4KiB - text/plain)
file #55455:  mybaseclass.m added by rdzman (173B - application/octet-stream)
file #55456:  constructor_bug.m added by rdzman (108B - application/octet-stream)
file #55457:  mysubclass.m added by rdzman (156B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel (Updated the item)
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by petter (Posted a comment)
  • -email is unavailable- added by rdzman (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-01-30 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 9.1.0 (current stable)
    2024-01-26 jwe StatusPatch Reviewed Ready For Test
    2024-01-26 mmuetzel Attached File- Added bug-65037-v2.patch, #55617
        StatusConfirmed Patch Reviewed
        Planned ReleaseNone 9.1.0 (current stable)
    2024-01-12 jwe Attached File- Added bug-65037.txt, #55557
    2024-01-12 jwe StatusNone Confirmed
    2023-12-15 rdzman Attached File- Added mybaseclass.m, #55455
        Attached File- Added constructor_bug.m, #55456
        Attached File- Added mysubclass.m, #55457

    Back to the top

    Powered by Savane 3.13-04b1.
    Corresponding source code