bugGNU Octave - Bugs: bug #45209, "echo on all" doesn't...

 
 

bug #45209: "echo on all" doesn't indent functions

Submitter:  Mike Miller <mtmiller>
Submitted:  Fri 29 May 2015 03:48:58 AM UTC
   
 
Category:  Interpreter Severity:  2 - Minor
Priority:  1 - Later Item Group:  Other
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 05 Jul 2017 07:49:19 PM UTC, comment #7: 
John W. Eaton <jwe>
Group administrator
Wed 05 Jul 2017 07:47:55 PM UTC, comment #6: 

I just checked in a series of changes that completely rework the implementation of echo.  Now the text comes directly from the source file (or command-line text) instead of being recreated from the parse tree.  Also, the text will appear only when commands are actually executing from a file and may be enabled or disabled from within functions.

I'm closing this report since now there is no indenting to do -- the text is just the source code, as it appears when Octave reads and parses it.

If there are additional problems with echo, please open a new bug report.

John W. Eaton <jwe>
Group administrator
Thu 28 Jan 2016 05:30:19 PM UTC, comment #5: 

I lowered the priority even further to "1 - Later".  I don't mind leaving it in since it is something that someone, someday, might want to fix.

Rik <rik5>
Group administrator
Mon 25 Jan 2016 03:44:18 AM UTC, comment #4: 

Thanks for that patch Rik.

Just as a reference point, I was recently using "echo on all" to debug a case where a variable mysteriously disappeared.  I wasn't at all bothered by the lack of indent.

Might I suggest a "Won't fix" / "closed" setting?  (I'm trying to stop the number of open bugs reaching 1000 :)

Lachlan Andrew <lachlan>
Sun 22 Nov 2015 04:13:02 PM UTC, comment #3: 

I pushed a small fix which avoids the assertion crash here (http://hg.savannah.gnu.org/hgweb/octave/rev/771d9fab5c24)

It would still be nice to get the sequence correct so that functions are indented correctly with "echo on all", but I'm not sure that this feature is used all that much.

I've re-titled this bug report and lowered the priority and severity.

Rik <rik5>
Group administrator
Sun 22 Nov 2015 04:09:24 AM UTC, comment #2: 

Apparently this has never worked correctly, although it didn't segfault.  The problem seems to be that curr_print_indent_level is not correctly incremented after the printing of the function header.  You can see this because all of the function body is at the same level of indent--that is 0--during the echoing.  It is only when another keyword which would increase indent, such as an 'if' statement, is encountered that the indent is increased.

I've attached pack2.m which is the file that I have been using for testing.  I instrumented the code in pt-pr-code.cc and I can see that the code path is probably not what was expected.  It seems that the path begins in ov-usr-fcn.cc in do_multi_index_op where print_code_function_header is called.  This, in turn, calls visit_octave_user_function_header in pt-pr-code.cc.  From reading the code in pt-pr-code.cc, it looks like the intended path may have been to call visit_octave_user_function().  That function in turn calls visit_octave_user_function_header() but it also has the code to call increment_indent_level().

Just to see if there was an easy fix, I added a call to increment_indent_level() at the end of visit_octave_user_function_header().  This correctly does the increment, but the next time the indent() function is called the value is back to 0.  I think this may be because the original call back in ov-usr-fcn.cc is sitting in an unwind/protect frame.  If that is the case, I'm not quite sure how to fix this crash.

I've added jwe to the CC list of the bug since this bug is beyond me.

Here is the test sequence I used.


more off
echo on all
pack2



(file #35518)

Rik <rik5>
Group administrator
Sat 30 May 2015 09:31:48 PM UTC, comment #1: 

I bisected and found the cause of this crash to be the recent change for bug #40462:

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

Part of this change moved where decrement_indent_level() is called so it is no longer strictly paired with increment_indent_level(), and somehow that is now allowing the indent level to go negative when ECHO_FUNCTIONS is used.

This does not impact the 4.0 stable branch.

Mike Miller <mtmiller>
Group Member
Fri 29 May 2015 03:48:58 AM UTC, original submission:  

Octave's parser panics and crashes when "echo on all" is used to display the contents of m-file functions as they are executed. Example with pack, which is a trivial noop function:


octave:1> pack
octave:2> more off
octave:3> echo on all
octave:4> pack
+ ## Copyright (C) 1999-2015 John W. Eaton
+ ##
+ ## 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/>.
+
+ ## -*- texinfo -*-
+ ## @deftypefn {Function File} {} pack ()
+ ## Consolidate workspace memory in @sc{matlab}.
+ ##
+ ## This function is provided for compatibility, but does nothing in Octave.
+ ## @seealso{clear}
+ ## @end deftypefn
+
+ ## Author: jwe
+
+ function pack ()
r��: ../../libinterp/parse-tree/pt-pr-code.cc:1205: void tree_print_code::indent(): Assertion `curr_print_indent_level >= 0' failed.
panic: Aborted -- stopping myself...
attempting to save variables to 'octave-workspace'...
save to 'octave-workspace' complete
Aborted


Mike Miller <mtmiller>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #35518:  pack2.m added by rik5 (97B - d2l/unknowntype)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by mtmiller (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 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-07-05 jwe StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2016-01-28 rik5 Priority3 - Low 1 - Later
    2015-11-22 rik5 Severity4 - Important 2 - Minor
        Priority5 - Normal 3 - Low
        Item GroupSegfault, Bus Error, etc. Other
        Summaryparser assertion failure and crash when &quot;echo on all&quot; is used "echo on all" doesn't indent functions
    2015-11-22 rik5 Attached File- Added pack2.m, #35518
    2015-11-22 rik5 Carbon-Copy- Added jwe
    2015-05-30 mtmiller Severity3 - Normal 4 - Important
        StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code