bugGNU Octave - Bugs: bug #45967, More consistent error message...

 
 

bug #45967: More consistent error message style for parse errors

Submitter:  Ceral Paquet <octavebugs>
Submitted:  Tue 15 Sep 2015 12:19:24 AM UTC
   
 
Category:  Interpreter Severity:  1 - Wish
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  rik5
Originator Name:  Open/Closed:  * Closed
Release:  * 4.0.0 Operating System:  * GNU/Linux
Fixed Release:  10.1.0 (current default) Planned Release:  10.1.0 (current default)
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 18 Jan 2024 10:45:27 PM UTC, comment #9: 

Final changeset https://hg.savannah.gnu.org/hgweb/octave/rev/dd46b69b0330 brings back reporting the full file name where the parse error occurs.

Marking bug as Fixed and closing report.

Rik <rik5>
Group administrator
Thu 18 Jan 2024 05:17:10 AM UTC, comment #8: 

Is function being parsed because it was called from the command line?  In that case, there is no execution stack other than the top-level workspace and that is normally omitted from the stack trace for errors anyway.

If you had a set of functions like


function foo ()
  bar ();
endfunction
function bar ()
  baz ();
endfunction
function baz ()
  x + y */+ 2 parse error here
end


Then I guess it would make sense to issue an error message that looked something like


error: syntax error near line 3 column 1 of /path/to/baz.m
error: called from
    bar.m at line 2 column 2
    foo.m at line 2 column 2


But I don't know why that is useful.  Evaluation and parse errors are different things.  Just having the file name, line number, and possibly the text of the line where the error occurs seems sufficient to me for diagnosing parse errors.  Where the to-be-parsed function is called from seems irrelevant to me.  I understand how a stack trace can help with understanding an evaluation error, but how would that info help with understanding a parse error?



John W. Eaton <jwe>
Group administrator
Thu 18 Jan 2024 01:18:06 AM UTC, comment #7: 

@jwe: I'm having trouble getting the program stack to be reported for parse errors.  The chain begins in oct-parse.yy which calls parse_error_with_id().  The action then shifts to error.cc.  The function parse_error_with_id() calls vparse_error_with_id() which calls error_1(). 


void
error_system::error_1 (const char *id, const char *fmt,
                       va_list args)
{
  std::string message = format_message (fmt, args);

  std::list<frame_info> stack_info;

  throw_error ("error", id, message);
}


The creation of the stack_info variable is unnecessary.  I'm not sure why the compiler doesn't throw a warning since it is unused.  In any case, this eventually calls throw_error().


void
error_system::throw_error (const std::string& err_type,
                           const std::string& id,
                           const std::string& message,
                           const std::list<frame_info>& stack_info_arg)
{
  std::list<frame_info> stack_info = stack_info_arg;

  if (stack_info.empty ())
    {
      tree_evaluator& tw = m_interpreter.get_evaluator ();

      stack_info = tw.backtrace_info ();

      // Print the error message only if it is different from the
      // previous one; makes the output more concise and readable.

      stack_info.unique ();
    }

  execution_exception ex (err_type, id, message, stack_info);

  throw_error (ex);
}


Since throw_error() is called without any stack_info the function calls tw.backtrace_info(), but this returns an empty stack.  Do you know why that would be?  Does parsing have to complete before the stack tree is populated?

If so, I can arrange for parser_error to call one of the other forms of error() and provide a stack to be printed.

Rik <rik5>
Group administrator
Thu 18 Jan 2024 12:49:52 AM UTC, comment #6: 

Of course this turns out to be harder than expected.  I pushed a changeset that fixes the failures in the test suite and also adds a new error ID "Octave:parse-error" which may be useful for categorizing errors.  See https://hg.savannah.gnu.org/hgweb/octave/rev/4f4092abe12f.

There is still more work to do.

Rik <rik5>
Group administrator
Wed 17 Jan 2024 06:15:09 PM UTC, comment #5: 

It broke some tests for
+verbose+
  bug-50014/bug-50014.tst ........................................ pass    2/11 
                                                                   FAIL    9
  classdef/classdef.tst .......................................... pass   34/38 
                                                                   FAIL    1
   
-verbose-

(see buildbots)

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Wed 17 Jan 2024 05:24:48 PM UTC, comment #4: 

We're early in development stage for Octave 10 so I am going to apply the patch to the development branch and see if there are problems going forward.

Rik <rik5>
Group administrator
Sat 13 Jan 2024 12:57:06 AM UTC, comment #3: 

This doesn't seem so hard to fix.  The code is in libinterp/parse-tree.  The code in question is yyerror.  Using grep, I only see 4 instances.


oct-parse.cc:1568:        yyerror (parser, YY_("syntax error: cannot back up")); \
oct-parse.cc:3055:      yyerror (parser, YY_("memory exhausted"));
oct-parse.cc:6316:      yyerror (parser, YY_("syntax error"));
oct-parse.cc:6427:  yyerror (parser, YY_("memory exhausted"));


This eventually calls this function in oct-parse.cc


void
base_parser::bison_error (const std::string& str, int err_line, int err_col)
{
  std::ostringstream output_buf;

  if (m_lexer.m_reading_fcn_file || m_lexer.m_reading_script_file
      || m_lexer.m_reading_classdef_file)
    output_buf << "parse error near line " << err_line
               << " of file " << m_lexer.m_fcn_file_full_name;
  else
    output_buf << "parse error:";

  if (str != "parse error")
    output_buf << "\n\n  " << str;

  output_buf << "\n\n";

  std::string curr_line;

  if (m_lexer.m_reading_fcn_file || m_lexer.m_reading_script_file
      || m_lexer.m_reading_classdef_file)
    curr_line = get_file_line (m_lexer.m_fcn_file_full_name, err_line);
  else
    curr_line = m_lexer.m_current_input_line;

  // Adjust the error column for display because it is 1-based in the
  // lexer for easier reporting.
  err_col--;

  if (! curr_line.empty ())
    {
      // FIXME: we could do better if we just cached lines from the
      // input file in a list.  See also functions for managing input
      // buffers in lex.ll.

      std::size_t len = curr_line.length ();

      if (curr_line[len-1] == '\n')
        curr_line.resize (len-1);

      // Print the line, maybe with a pointer near the error token.

      output_buf << ">>> " << curr_line << "\n";

      if (err_col == 0)
        err_col = len;

      for (int i = 0; i < err_col + 3; i++)
        output_buf << " ";

      output_buf << "^";
    }

  output_buf << "\n";

  m_parse_error_msg = output_buf.str ();
}


Octave apparently has the column information so it seems like it would be helpful to print it.

I worked up a small changeset which is attached.  Using it, the output is


octave:1> myfoo
error: syntax error near line 3, column 4
error: called from
    myfoo at line 3 column 4
octave:2> mybar
error: 'i_am_a_bug' undefined near line 3, column 1
error: called from
    mybar at line 3 column 1
octave:3> i am a bug
error: syntax error

>>> i am a bug
       ^
octave:3>




(file #55558)

Rik <rik5>
Group administrator
Wed 16 Sep 2015 12:16:26 PM UTC, comment #2: 

I think the "parse error" messages come from the yyerror (or octave_error) function in the parser and that's separate from the error handling code that is used when code is evaluated in Octave.

It does make sense to me to at least unify the way error messages are displayed.

John W. Eaton <jwe>
Group administrator
Tue 15 Sep 2015 02:10:36 AM UTC, comment #1: 

This is interesting. My first instinct is to say that they are really two different kinds of errors, but I realized that command syntax allows "sentences" like the first example to work just fine. Maybe "i" is a special case in that it is both syntax and a built-in function named "i".

But compare


>> i am a bug
parse error:

  syntax error

>>> i am a bug
       ^

>> fft am a bug
error: invalid conversion from string to real scalar


Both "i" and "fft" are built-in functions, but with "fft" it does the normal conversion from command to function call, and the error is different.

Perhaps it is a bug that "i am a bug" is flagged as a syntax error rather than an invalid function call to the function named "i", but that's about it.

If your function were


function f ()
  1 +
endfunction


the same "parse error" would be thrown, because it is illegal syntax, not just an invalid function call or reference to an unknown variable.

Mike Miller <mtmiller>
Group Member
Tue 15 Sep 2015 12:19:24 AM UTC, original submission:  

function myfoo()

i am a bug

endfunction


 
function mybar()

i_am_a_bug

endfuction


-----------------


>> myfoo

parse error near line 3 of file /home/ceral/octave/myfoo.m

  syntax error

>>> i am a bug

       ^

>> mybar

error: 'i_am_a_bug' undefined near line 3 column 1
error: called from
    mybar at line 3 column 1



Would it be possible to use the same style for these two types of error? Perhaps the latter is slightly tidier. How about:

>> myfoo

error: syntax error near line 3 column 1
error: called from
    myfoo.m at line 3 column 1


What would be awesome is to have the final line be clickable and take you to that file and line in the Editor.

Ceral Paquet <octavebugs>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #55558:  bug45967.cset added by rik5 (4KiB - 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 dasergatskov (Posted a comment)
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by octavebugs (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 13 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-01-18 rik5 StatusIn Progress Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 10.1.0 (current default)
    2024-01-18 rik5 StatusReady For Test In Progress
        Assigned toNone rik5
    2024-01-17 rik5 StatusPatch Submitted Ready For Test
        Planned ReleaseNone 10.1.0 (current default)
    2024-01-13 rik5 StatusNeed Info Patch Submitted
    2024-01-13 rik5 Attached File- Added bug45967.cset, #55558
    2015-09-15 mtmiller CategoryNone Interpreter
        Severity3 - Normal 1 - Wish
        Item GroupNone Feature Request
        StatusNone Need Info

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code