bugGNU Octave - Bugs: bug #59661, classdef method call fails with...

 
 

bug #59661: classdef method call fails with "octave_base_value::function_value(): wrong type argument '<unknown type>'"

Submitter:  Sébastien Villemot <svillemot>
Submitted:  Fri 11 Dec 2020 05:13:57 PM UTC
   
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 6.1.0 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 21 Dec 2020 01:08:25 PM UTC, comment #12: 

The example here and the test suite for the stk package are running without errors for me now.

Closing as fixed.

Markus Mützel <mmuetzel>
Group administrator
Sun 20 Dec 2020 05:54:44 PM UTC, comment #11: 
Markus Mützel <mmuetzel>
Group administrator
Tue 15 Dec 2020 07:58:11 PM UTC, comment #10: 

That's strange, I'm certain I verified my change, but I must have made a mistake in my testing.

I also think that in this context, we should be looking for a method associated with the dispatch type.  But find_method is returning an octave_classdef_meta object instead of an octave_function object.

For now, I guess we should revert the change I made and use find_function instead of find_method.  I'll do that soon unless someone gets there first.

Thanks.

John W. Eaton <jwe>
Group administrator
Tue 15 Dec 2020 06:36:40 PM UTC, comment #9: 

Oops. I didn't use the correct style consistently.
I'm attaching an updated patch which should hopefully better follow Octave's style. Otherwise, it is the same as the previous patch.

(file #50481)

Markus Mützel <mmuetzel>
Group administrator
Tue 15 Dec 2020 06:25:18 PM UTC, comment #8: 

The attached patch adds a BIST with a simpler class that exhibits the same error.

(file #50480)

Markus Mützel <mmuetzel>
Group administrator
Tue 15 Dec 2020 05:39:10 PM UTC, comment #7: 

Confirmed here.

If I revert to the original changeset, the example in comment #0 works again:

diff -r ed1a8af4242f libinterp/octave-value/ov-fcn-handle.cc
--- a/libinterp/octave-value/ov-fcn-handle.cc        Tue Dec 15 17:08:43 2020 +0100
+++ b/libinterp/octave-value/ov-fcn-handle.cc        Tue Dec 15 18:35:51 2020 +0100
@@ -449,7 +449,7 @@
       // Cache this value so that the pointer will be valid as long as the
       // function handle object is valid.

-      m_fcn = symtab.find_method (m_name, m_dispatch_class);
+      m_fcn = symtab.find_function (m_name, octave_value_list ());

       return m_fcn.is_defined () ? m_fcn.function_value () : nullptr;
     }


But again: I'm new to that part of the code. I'm not sure if that change is always correct.

Markus Mützel <mmuetzel>
Group administrator
Tue 15 Dec 2020 04:23:11 PM UTC, comment #6: 

Actually there seems to be another problem.

When I apply Markus’ patch on top of a pristine 6.1.0 tarball, then I get the correct result (i.e. same behaviour as 5.2.0).

However, when I compile the current tip of the stable branch (which includes Markus’ patch, plus various other bugfixes), I still get an error, but the message is now different:


octave:1> d1 = dates('1950Q1', '1950Q2');
octave:2> d2 = dates('1950Q2');
octave:3> d1 == d2
error: matrix cannot be indexed with .
error: called from
    eq at line 27 column 1
    eq at line 37 column 7


Sébastien Villemot <svillemot>
Tue 15 Dec 2020 03:31:29 PM UTC, comment #5: 

Of course it's perfectly fine if you use the test case that I attached to this report. It is free software. But it’s certainly not a minimal test case, it needs some more work to reach that state.

Sébastien Villemot <svillemot>
Mon 14 Dec 2020 09:47:13 PM UTC, comment #4: 

Or maybe it would be better to use a simplified test?  Do we have one?

For now, I pushed the following changeset on stable and merged with default:

http://hg.savannah.gnu.org/hgweb/octave/rev/e359e0fcd6e7

John W. Eaton <jwe>
Group administrator
Mon 14 Dec 2020 08:58:59 PM UTC, comment #3: 

Instead of calling symbol_table::find_function, I think we should be calling find_method with the values of m_name and m_dispatch_class that are stored in the class_simple_fcn_handle object.  My modified change is attached.

Sébastien, is it OK to use your example as a test?


(file #50461)

John W. Eaton <jwe>
Group administrator
Mon 14 Dec 2020 05:07:33 PM UTC, comment #2: 

Thanks. I confirm that your patch fixes the testcase I attached to this bug, and also many other similar error messages that I am experiencing in a classdef-intensive testsuite.

Sébastien Villemot <svillemot>
Sun 13 Dec 2020 04:04:08 PM UTC, comment #1: 

Confirmed with Octave 6.1.0 on Windows 10 and with a build from the current default branch on Ubuntu 20.10.

I set a breakpoint in `err_wrong_type_arg` where this error is thrown. The start of the backtrace from that point is:

(gdb) bt
#0  0x00007ffff6905290 in err_wrong_type_arg(char const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@plt ()
    at /home/osboxes/Documents/Repositories/Octave/octave-1/.build/libinterp/.libs/liboctinterp.so.8
#1  0x00007ffff6d96849 in octave_base_value::function_value(bool) (this=0x7ffff7a0a5d0 <octave_value::nil_rep()::nr>, silent=false) at ../libinterp/octave-value/ov-base.cc:930
#2  0x00007ffff6efe065 in octave_value::function_value(bool) const (this=0x7fffcd14a240, silent=false) at ../libinterp/octave-value/ov.cc:1775
#3  0x00007ffff6e14b9a in octave::class_simple_fcn_handle::function_value(bool) (this=0x7fffcd14a1f0) at ../libinterp/octave-value/ov-fcn-handle.cc:439
#4  0x00007ffff6e1351d in octave_fcn_handle::function_value(bool) (this=0x7fffcd147460) at ../libinterp/octave-value/ov-fcn-handle.h:264
#5  0x00007ffff6efe065 in octave_value::function_value(bool) const (this=0x7fffdaff25d8, silent=false) at ../libinterp/octave-value/ov.cc:1775
#6  0x00007ffff709a503 in Fbsxfun(octave::interpreter&, octave_value_list const&, int) (interp=..., args=...) at ../libinterp/corefcn/bsxfun.cc:371
#7  0x00007ffff6dae3c8 in octave_builtin::execute(octave::tree_evaluator&, int, octave_value_list const&) (this=0x7fffccebf970, tw=..., nargout=1, args=...) at ../libinterp/octave-value/ov-builtin.cc:65


I'm not sure if I got the picture correctly. But looking at the code, I think the correct thing might happen if `octave::class_simple_fcn_handle::function_value` would do the same as `octave::simple_fcn_handle::function_value`.

With the attached patch the example in comment #0 works for me and all BISTs still pass. I don't understand that part of the code well enough though to estimate if this is the correct thing to do.

(file #50452)

Markus Mützel <mmuetzel>
Group administrator
Fri 11 Dec 2020 05:13:57 PM UTC, original submission:  

Hi,

I attach a testcase for a classdef-related regression in 6.1.0:


octave:1> d1 = dates('1950Q1', '1950Q2');
octave:2> d2 = dates('1950Q2');
octave:3> d1 == d2
error: octave_base_value::function_value(): wrong type argument '<unknown type>'


Under 5.2.0, the result is:

octave:3> d1 == d2
ans =

  0
  1


Note that thhe error message is the same as case b) of bug #59340.

Sébastien Villemot <svillemot>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #50461:  bug-59661-diffs.txt added by jwe (1KiB - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by mmuetzel (Updated the item)
  • -email is unavailable- added by svillemot (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
    2020-12-21 mmuetzel StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2020-12-20 mmuetzel StatusIn Progress Ready For Test
    2020-12-15 mmuetzel Attached File- Added bug59661_test_case_v2.patch, #50481
    2020-12-15 mmuetzel Attached File- Added bug59661_test_case.patch, #50480
    2020-12-14 jwe StatusPatch Submitted In Progress
    2020-12-14 jwe Attached File- Added bug-59661-diffs.txt, #50461
        StatusConfirmed Patch Submitted
    2020-12-13 mmuetzel Attached File- Added bug59661_function_handle.patch, #50452
        StatusNone Confirmed
        Operating SystemGNU/Linux Any
    2020-12-11 svillemot Attached File- Added classdef-6.1.0-regression.tar.gz, #50439

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code