Fix line number reporting in new parser

builtin.cpp

@@ -1729,7 +1729,7 @@ static int builtin_pwd(parser_t &parser, wchar_t **argv)
 }
 
 /** Adds a function to the function set. It calls into function.cpp to perform any heavy lifting. */
-int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstring &contents, wcstring *out_err)
+int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstring &contents, int definition_line_offset, wcstring *out_err)
 {
     assert(out_err != NULL);
 
@@ -2027,8 +2027,7 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr
 
         d.definition = contents.c_str();
 
-        // TODO: fix def_offset inside function_add
+        function_add(d, parser, definition_line_offset);
-        function_add(d, parser);
     }
 
     return res;

builtin.h

@@ -179,7 +179,7 @@ const wchar_t *builtin_complete_get_temporary_buffer();
 wcstring builtin_help_get(parser_t &parser, const wchar_t *cmd);
 
 /** Defines a function, like builtin_function. Returns 0 on success. args should NOT contain 'function' as the first argument. */
-int define_function(parser_t &parser, const wcstring_list_t &args, const wcstring &contents, wcstring *out_err);
+int define_function(parser_t &parser, const wcstring_list_t &args, const wcstring &contents, int definition_line_offset, wcstring *out_err);
 
 
 #endif

common.cpp

@@ -713,6 +713,11 @@ void debug(int level, const char *msg, ...)
     errno = errno_old;
 }
 
+void print_stderr(const wcstring &str)
+{
+    fprintf(stderr, "%ls\n", str.c_str());
+}
+
 
 void debug_safe(int level, const char *msg, const char *param1, const char *param2, const char *param3, const char *param4, const char *param5, const char *param6, const char *param7, const char *param8, const char *param9, const char *param10, const char *param11, const char *param12)
 {

common.h

@@ -732,6 +732,9 @@ ssize_t read_loop(int fd, void *buff, size_t count);
 void debug(int level, const char *msg, ...);
 void debug(int level, const wchar_t *msg, ...);
 
+/** Writes a string to stderr, followed by a newline */
+void print_stderr(const wcstring &str);
+
 /**
    Replace special characters with backslash escape sequences. Newline is
    replaced with \n, etc.

exec.cpp

@@ -1430,8 +1430,7 @@ void exec_job(parser_t &parser, job_t *j)
                 if (g_log_forks)
                 {
                     const wchar_t *file = reader_current_filename();
-                    const wchar_t *func = parser_t::principal_parser().is_function();
+                    printf("fork #%d: forking for '%s' in '%ls'\n", g_fork_count, actual_cmd, file ? file : L"");
-                    printf("fork #%d: forking for '%s' in '%ls:%ls'\n", g_fork_count, actual_cmd, file ? file : L"", func ? func : L"?");
 
                     fprintf(stderr, "IO chain for %s:\n", actual_cmd);
                     io_print(process_net_io_chain);

function.cpp

@@ -175,7 +175,7 @@ function_info_t::function_info_t(const function_info_t &data, const wchar_t *fil
 {
 }
 
-void function_add(const function_data_t &data, const parser_t &parser)
+void function_add(const function_data_t &data, const parser_t &parser, int definition_line_offset)
 {
     ASSERT_IS_MAIN_THREAD();
 
@@ -189,13 +189,7 @@ void function_add(const function_data_t &data, const parser_t &parser)
     /* Create and store a new function */
     const wchar_t *filename = reader_current_filename();
 
-    int def_offset = -1;
+    const function_map_t::value_type new_pair(data.name, function_info_t(data, filename, definition_line_offset, is_autoload));
-    if (parser.current_block() != NULL)
-    {
-        def_offset = parser.line_number_of_character_at_offset(parser.current_block()->tok_pos);
-    }
-
-    const function_map_t::value_type new_pair(data.name, function_info_t(data, filename, def_offset, is_autoload));
     loaded_functions.insert(new_pair);
 
     /* Add event handlers */

function.h

@@ -92,8 +92,8 @@ public:
 */
 void function_init();
 
-/** Add a function. */
+/** Add a function. definition_line_offset is the line number of the function's definition within its source file */
-void function_add(const function_data_t &data, const parser_t &parser);
+void function_add(const function_data_t &data, const parser_t &parser, int definition_line_offset = 0);
 
 /** Removes a function from our internal table, returning true if it was found and false if not */
 bool function_remove_ignore_autoload(const wcstring &name);

parse_execution.cpp

@@ -381,8 +381,9 @@ parse_execution_result_t parse_execution_context_t::run_function_statement(const
     if (result == parse_execution_success)
     {
         const wcstring contents_str = get_source(contents);
+        int definition_line_offset = this->line_offset_of_node_at_offset(this->get_offset(contents));
         wcstring error_str;
-        int err = define_function(*parser, argument_list, contents_str, &error_str);
+        int err = define_function(*parser, argument_list, contents_str, definition_line_offset, &error_str);
         proc_set_last_status(err);
 
         if (! error_str.empty())
@@ -1523,29 +1524,29 @@ parse_execution_result_t parse_execution_context_t::eval_node_at_offset(node_off
     return status;
 }
 
-int parse_execution_context_t::get_current_line_number()
+int parse_execution_context_t::line_offset_of_node_at_offset(node_offset_t requested_index)
 {
     /* If we're not executing anything, return -1 */
-    if (this->executing_node_idx == NODE_OFFSET_INVALID)
+    if (requested_index == NODE_OFFSET_INVALID)
     {
         return -1;
     }
 
     /* If for some reason we're executing a node without source, return -1 */
-    const parse_node_t &node = tree.at(this->executing_node_idx);
+    const parse_node_t &node = tree.at(requested_index);
     if (! node.has_source())
     {
         return -1;
     }
     
     /* Count the number of newlines, leveraging our cache */
-    const size_t offset = tree.at(this->executing_node_idx).source_start;
+    const size_t offset = tree.at(requested_index).source_start;
     assert(offset <= src.size());
 
     /* Easy hack to handle 0 */
     if (offset == 0)
     {
-        return 1;
+        return 0;
     }
     
     /* We want to return (one plus) the number of newlines at offsets less than the given offset. cached_lineno_count is the number of newlines at indexes less than cached_lineno_offset. */
@@ -1575,5 +1576,17 @@ int parse_execution_context_t::get_current_line_number()
         }
         cached_lineno_offset = offset;
     }
-    return cached_lineno_count + 1;
+    return cached_lineno_count;
+}
+
+int parse_execution_context_t::get_current_line_number()
+{
+    int line_number = -1;
+    int line_offset = this->line_offset_of_node_at_offset(this->executing_node_idx);
+    if (line_offset >= 0)
+    {
+        /* The offset is 0 based; the number is 1 based */
+        line_number = line_offset + 1;
+    }
+    return line_number;
 }

parse_execution.h

@@ -107,13 +107,16 @@ private:
     parse_execution_result_t run_job_list(const parse_node_t &job_list_node, const block_t *associated_block);
     parse_execution_result_t populate_job_from_job_node(job_t *j, const parse_node_t &job_node, const block_t *associated_block);
 
+    /* Returns the line number of the node at the given index, indexed from 0. Not const since it touches cached_lineno_offset */
+    int line_offset_of_node_at_offset(node_offset_t idx);
+
 public:
     parse_execution_context_t(const parse_node_tree_t &t, const wcstring &s, parser_t *p, int initial_eval_level);
 
     /* Returns the current eval level */
     int current_eval_level() const { return eval_level; }
     
-    /* Returns the current line number. Not const since it touches cached_lineno_offset */
+    /* Returns the current line number, indexed from 1. Not const since it touches cached_lineno_offset */
     int get_current_line_number();
 
     /* Start executing at the given node offset. Returns 0 if there was no error, 1 if there was an error */

parser.cpp

@@ -242,7 +242,12 @@ void parser_t::push_block(block_t *new_current)
 {
     const enum block_type_t type = new_current->type();
     new_current->src_lineno = parser_t::get_lineno();
-    new_current->src_filename = parser_t::current_filename()?intern(parser_t::current_filename()):0;
+
+    const wchar_t *filename = parser_t::current_filename();
+    if (filename != NULL)
+    {
+        new_current->src_filename = intern(filename);
+    }
 
     const block_t *old_current = this->current_block();
     if (old_current && old_current->skip)
@@ -325,6 +330,27 @@ const wchar_t *parser_t::get_block_desc(int block) const
     return _(UNKNOWN_BLOCK);
 }
 
+wcstring parser_t::block_stack_description() const
+{
+    wcstring result;
+    size_t idx = this->block_count();
+    size_t spaces = 0;
+    while (idx--)
+    {
+        if (spaces > 0)
+        {
+            result.push_back(L'\n');
+        }
+        for (size_t j=0; j < spaces; j++)
+        {
+            result.push_back(L' ');
+        }
+        result.append(this->block_at_index(idx)->description());
+        spaces++;
+    }
+    return result;
+}
+
 const block_t *parser_t::block_at_index(size_t idx) const
 {
     /* 0 corresponds to the last element in our vector */
@@ -732,6 +758,11 @@ const wchar_t *parser_t::is_function() const
             result = fb->name.c_str();
             break;
         }
+        else if (b->type() == SOURCE)
+        {
+            /* If a function sources a file, obviously that function's offset doesn't contribute */
+            break;
+        }
     }
     return result;
 }
@@ -743,6 +774,14 @@ int parser_t::get_lineno() const
     if (! execution_contexts.empty())
     {
         lineno = execution_contexts.back()->get_current_line_number();
+
+        /* If we are executing a function, we have to add in its offset */
+        const wchar_t *function_name = is_function();
+        if (function_name != NULL)
+        {
+            lineno += function_get_definition_offset(function_name);
+        }
+
     }
     return lineno;
 }
@@ -764,12 +803,16 @@ const wchar_t *parser_t::current_filename() const
     for (size_t i=0; i < this->block_count(); i++)
     {
         const block_t *b = this->block_at_index(i);
-        /* Note that we deliberately skip FUNCTION_CALL_NO_SHADOW here, because the only such functions in wide use are '.' and eval, and we don't want to report those */
+        if (b->type() == FUNCTION_CALL || b->type() == FUNCTION_CALL_NO_SHADOW)
-        if (b->type() == FUNCTION_CALL)
         {
             const function_block_t *fb = static_cast<const function_block_t *>(b);
             return function_get_definition_file(fb->name);
         }
+        else if (b->type() == SOURCE)
+        {
+            const source_block_t *sb = static_cast<const source_block_t *>(b);
+            return sb->source_file;
+        }
     }
 
     /* We query a global array for the current file name, but only do that if we are the principal parser */
@@ -1045,6 +1088,8 @@ int parser_t::eval_new_parser(const wcstring &cmd, const io_chain_t &io, enum bl
         return 1;
     }
 
+    //print_stderr(block_stack_description());
+
 
     /* Determine the initial eval level. If this is the first context, it's -1; otherwise it's the eval level of the top context. This is sort of wonky because we're stitching together a global notion of eval level from these separate objects. A better approach would be some profile object that all contexts share, and that tracks the eval levels on its own. */
     int exec_eval_level = (execution_contexts.empty() ? -1 : execution_contexts.back()->current_eval_level());
@@ -1395,7 +1440,6 @@ void parser_t::get_backtrace(const wcstring &src, const parse_error_list_t &erro
 block_t::block_t(block_type_t t) :
     block_type(t),
     skip(),
-    had_command(),
     tok_pos(),
     node_offset(NODE_OFFSET_INVALID),
     loop_status(),
@@ -1411,6 +1455,83 @@ block_t::~block_t()
 {
 }
 
+wcstring block_t::description() const
+{
+    wcstring result;
+    switch (this->type())
+    {
+        case WHILE:
+            result.append(L"while");
+            break;
+
+        case FOR:
+            result.append(L"for");
+            break;
+
+        case IF:
+            result.append(L"if");
+            break;
+
+        case FUNCTION_DEF:
+            result.append(L"function_def");
+            break;
+
+        case FUNCTION_CALL:
+            result.append(L"function_call");
+            break;
+
+        case FUNCTION_CALL_NO_SHADOW:
+            result.append(L"function_call_no_shadow");
+            break;
+
+        case SWITCH:
+            result.append(L"switch");
+            break;
+
+        case FAKE:
+            result.append(L"fake");
+            break;
+
+        case SUBST:
+            result.append(L"substitution");
+            break;
+
+        case TOP:
+            result.append(L"top");
+            break;
+
+        case BEGIN:
+            result.append(L"begin");
+            break;
+
+        case SOURCE:
+            result.append(L"source");
+            break;
+
+        case EVENT:
+            result.append(L"event");
+            break;
+
+        case BREAKPOINT:
+            result.append(L"breakpoint");
+            break;
+
+        default:
+            append_format(result, L"unknown type %ld", (long)this->type());
+            break;
+    }
+
+    if (this->src_lineno >= 0)
+    {
+        append_format(result, L" (line %d)", this->src_lineno);
+    }
+    if (this->src_filename != NULL)
+    {
+        append_format(result, L" (file %ls)", this->src_filename);
+    }
+    return result;
+}
+
 /* Various block constructors */
 
 if_block_t::if_block_t() : block_t(IF)

parser.h

@@ -84,8 +84,10 @@ public:
         return this->block_type;
     }
 
+    /** Description of the block, for debugging */
+    wcstring description() const;
+
     bool skip; /**< Whether execution of the commands in this block should be skipped */
-    bool had_command; /**< Set to non-zero once a command has been executed in this block */
     int tok_pos; /**< The start index of the block */
 
     node_offset_t node_offset; /* Offset of the node */
@@ -96,7 +98,7 @@ public:
     /** The job that is currently evaluated in the specified block. */
     job_t *job;
 
-    /** Name of file that created this block */
+    /** Name of file that created this block. This string is intern'd. */
     const wchar_t *src_filename;
 
     /** Line number where this block was created */
@@ -272,6 +274,12 @@ private:
     /** The list of blocks, allocated with new. It's our responsibility to delete these */
     std::vector<block_t *> block_stack;
 
+    /** Gets a description of the block stack, for debugging */
+    wcstring block_stack_description() const;
+
+    /** List of profile items, allocated with new */
+    std::vector<profile_item_t*> profile_items;
+
     /* No copying allowed */
     parser_t(const parser_t&);
     parser_t& operator=(const parser_t&);
@@ -287,9 +295,6 @@ private:
     /** Adds a job to the beginning of the job list. */
     void job_add(job_t *job);
 
-public:
-    std::vector<profile_item_t*> profile_items;
-
     /**
        Returns the name of the currently evaluated function if we are
        currently evaluating a function, null otherwise. This is tested by
@@ -298,6 +303,8 @@ public:
     */
     const wchar_t *is_function() const;
 
+public:
+
     /** Get the "principal" parser, whatever that is */
     static parser_t &principal_parser();