Merge pull request #1297 from Thirumalai-Shaktivel/extend_loc_manager_1

Extend Location manger to handle multi-file import errors

Author: certik

.gitignore

@@ -62,7 +62,7 @@ share/jupyter/kernels/fortran/kernel.json
 src/runtime/*.o.empty.c
 python_ast.py
 python_ast.h
-ser.txt
+input.txt
 integration_tests/py_*
 integration_tests/b1/*
 integration_tests/b2/*
@@ -72,6 +72,7 @@ integration_tests/b5/*
 integration_tests/b6/*
 integration_tests/_lpython-tmp-test-*
 inst/bin/*
+*.tmp
 *.tlog
 *.filters
 *.obj

src/bin/lpython.cpp

@@ -375,9 +375,14 @@ class ASRToLLVMVisitor : public ASR::BaseVisitor<ASRToLLVMVisitor>
     void debug_get_line_column(const uint32_t &loc_first,
             uint32_t &line, uint32_t &column) {
         LocationManager lm;
-        lm.in_filename = infile;
-        lm.init_simple(LFortran::read_file(infile));
-        lm.pos_to_linecol(lm.output_to_input_pos(loc_first, false), line, column);
+        LocationManager::FileLocations fl;
+        fl.in_filename = infile;
+        lm.files.push_back(fl);
+        std::string input = LFortran::read_file(infile);
+        lm.init_simple(input);
+        lm.file_ends.push_back(input.size());
+        lm.pos_to_linecol(lm.output_to_input_pos(loc_first, false),
+            line, column, fl.in_filename);
     }
 
     template <typename T>

src/libasr/codegen/asr_to_llvm.cpp

@@ -61,20 +61,19 @@ bool Diagnostics::has_error() const {
     return false;
 }
 
-std::string Diagnostics::render(const std::string &input,
-        const LocationManager &lm, const CompilerOptions &compiler_options) {
+std::string Diagnostics::render(LocationManager &lm,
+        const CompilerOptions &compiler_options) {
     std::string out;
     for (auto &d : this->diagnostics) {
         if (compiler_options.no_warnings && d.level != Level::Error) {
             continue;
         }
         if (compiler_options.error_format == "human") {
-            out += render_diagnostic_human(d, input, lm,
-                compiler_options.use_colors,
+            out += render_diagnostic_human(d, lm, compiler_options.use_colors,
                 compiler_options.show_stacktrace);
             if (&d != &this->diagnostics.back()) out += "\n";
         } else if (compiler_options.error_format == "short") {
-            out += render_diagnostic_short(d, input, lm);
+            out += render_diagnostic_short(d, lm);
         } else {
             throw LCompilersException("Error format not supported.");
         }
@@ -119,49 +118,47 @@ std::string get_line(std::string str, int n)
     return line;
 }
 
-void populate_span(diag::Span &s, const LocationManager &lm,
-        const std::string &input) {
+void populate_span(diag::Span &s, const LocationManager &lm) {
     lm.pos_to_linecol(lm.output_to_input_pos(s.loc.first, false),
-        s.first_line, s.first_column);
+        s.first_line, s.first_column, s.filename);
     lm.pos_to_linecol(lm.output_to_input_pos(s.loc.last, true),
-        s.last_line, s.last_column);
-    s.filename = lm.in_filename;
+        s.last_line, s.last_column, s.filename);
+    std::string input;
+    read_file(s.filename, input);
     for (uint32_t i = s.first_line; i <= s.last_line; i++) {
         s.source_code.push_back(get_line(input, i));
     }
     LFORTRAN_ASSERT(s.source_code.size() > 0)
 }
 
 // Loop over all labels and their spans, populate all of them
-void populate_spans(diag::Diagnostic &d, const LocationManager &lm,
-        const std::string &input) {
+void populate_spans(diag::Diagnostic &d, const LocationManager &lm) {
     for (auto &l : d.labels) {
         for (auto &s : l.spans) {
-            populate_span(s, lm, input);
+            populate_span(s, lm);
         }
     }
 }
 
 // Fills Diagnostic with span details and renders it
-std::string render_diagnostic_human(Diagnostic &d, const std::string &input,
-        const LocationManager &lm, bool use_colors, bool show_stacktrace) {
+std::string render_diagnostic_human(Diagnostic &d, const LocationManager &lm,
+        bool use_colors, bool show_stacktrace) {
     std::string out;
     if (show_stacktrace) {
         out += error_stacktrace(d.stacktrace);
     }
     // Convert to line numbers and get source code strings
-    populate_spans(d, lm, input);
+    populate_spans(d, lm);
     // Render the message
     out += render_diagnostic_human(d, use_colors);
     return out;
 }
 
 // Fills Diagnostic with span details and renders it
-std::string render_diagnostic_short(Diagnostic &d, const std::string &input,
-        const LocationManager &lm) {
+std::string render_diagnostic_short(Diagnostic &d, const LocationManager &lm) {
     std::string out;
     // Convert to line numbers and get source code strings
-    populate_spans(d, lm, input);
+    populate_spans(d, lm);
     // Render the message
     out += render_diagnostic_short(d);
     return out;
@@ -245,11 +242,24 @@ std::string render_diagnostic_human(const Diagnostic &d, bool use_colors) {
                 }
                 // and start a new one:
                 s0 = s2;
+                if (s0.filename != s.filename) {
+                    out << std::endl;
+                    // TODO: print the primary line+column here, not the first label:
+                    out << std::string(line_num_width, ' ') << blue_bold;
+                    out << "-->" << reset << " " << s0.filename << ":";
+                    out << s0.first_line << ":" << s0.first_column;
+                    if (s0.first_line != s0.last_line) {
+                        out << " - " << s0.last_line << ":" << s0.last_column;
+                    }
+                    out << std::endl;
+                }
+
                 if (s0.first_line == s0.last_line) {
                     out << std::string(line_num_width+1, ' ') << blue_bold << "|"
                         << reset << std::endl;
                     std::string line = s0.source_code[0];
                     std::replace(std::begin(line), std::end(line), '\t', ' ');
+                    line.erase(std::remove(line.begin(), line.end(), '\r'), line.end());
                     out << blue_bold << std::setw(line_num_width)
                         << std::to_string(s0.first_line) << " |" << reset << " "
                         << line << std::endl;
@@ -263,6 +273,7 @@ std::string render_diagnostic_human(const Diagnostic &d, bool use_colors) {
                             << reset << std::endl;
                         std::string line = s0.source_code[0];
                         std::replace(std::begin(line), std::end(line), '\t', ' ');
+                        line.erase(std::remove(line.begin(), line.end(), '\r'), line.end());
                         out << blue_bold << std::setw(line_num_width)
                             << std::to_string(s0.first_line) << " |" << reset << " "
                             << "   " + line << std::endl;
@@ -281,6 +292,7 @@ std::string render_diagnostic_human(const Diagnostic &d, bool use_colors) {
                             << reset << std::endl;
                         line = s0.source_code[s0.source_code.size()-1];
                         std::replace(std::begin(line), std::end(line), '\t', ' ');
+                        line.erase(std::remove(line.begin(), line.end(), '\r'), line.end());
                         out << blue_bold << std::setw(line_num_width)
                             << std::to_string(s0.last_line) << " |" << reset << " "
                             << "   " + line << std::endl;

src/libasr/diagnostics.cpp

@@ -115,9 +115,7 @@ struct Diagnostic {
 struct Diagnostics {
     std::vector<Diagnostic> diagnostics;
 
-    // Render nice error messages using all the information we have
-    std::string render(const std::string &input,
-            const LocationManager &lm, const CompilerOptions &compiler_options);
+    std::string render(LocationManager &lm, const CompilerOptions &compiler_options);
 
     // Renders the error message using only the information in Diagnostics
     std::string render2();
@@ -214,10 +212,9 @@ std::string render_diagnostic_human(const Diagnostic &d, bool use_colors);
 std::string render_diagnostic_short(const Diagnostic &d);
 
 // Fills Diagnostic with span details and renders it
-std::string render_diagnostic_human(Diagnostic &d, const std::string &input,
-        const LocationManager &lm, bool use_colors, bool show_stacktrace);
-std::string render_diagnostic_short(Diagnostic &d, const std::string &input,
-        const LocationManager &lm); 
+std::string render_diagnostic_human(Diagnostic &d, const LocationManager &lm,
+    bool use_colors, bool show_stacktrace);
+std::string render_diagnostic_short(Diagnostic &d, const LocationManager &lm);
 /**
  * @brief Convert diagnostic `Level` i.e. severity to string and color accordingly.
  *

src/libasr/diagnostics.h

@@ -94,50 +94,64 @@ struct LocationManager {
     //
     //
     //
-    std::vector<uint32_t> out_start; // consecutive intervals in the output code
-    std::vector<uint32_t> in_start; // start + size in the original code
-    std::vector<uint32_t> in_newlines; // position of all \n in the original code
+    struct FileLocations {
+        std::vector<uint32_t> out_start; // consecutive intervals in the output code
+        std::vector<uint32_t> in_start; // start + size in the original code
+        std::vector<uint32_t> in_newlines; // position of all \n in the original code
 
-    // For preprocessor (if preprocessor==true).
-    // TODO: design a common structure, that also works with #include, that
-    // has these mappings for each file
-    bool preprocessor = false;
-    std::string in_filename;
-    uint32_t current_line=0;
-    std::vector<uint32_t> out_start0; // consecutive intervals in the output code
-    std::vector<uint32_t> in_start0; // start + size in the original code
-    std::vector<uint32_t> in_size0; // Size of the `in` interval
-    std::vector<uint32_t> interval_type0; // 0 .... 1:1; 1 ... many to many;
-    std::vector<uint32_t> in_newlines0; // position of all \n in the original code
-//    std::vector<uint32_t> filename_id; // file name for each interval, ID
-//    std::vector<std::string> filenames; // filenames lookup for an ID
+        // For preprocessor (if preprocessor==true).
+        // TODO: design a common structure, that also works with #include, that
+        // has these mappings for each file
+        bool preprocessor = false;
+        std::string in_filename;
+        uint32_t current_line=0;
+        std::vector<uint32_t> out_start0; // consecutive intervals in the output code
+        std::vector<uint32_t> in_start0; // start + size in the original code
+        std::vector<uint32_t> in_size0; // Size of the `in` interval
+        std::vector<uint32_t> interval_type0; // 0 .... 1:1; 1 ... many to many;
+        std::vector<uint32_t> in_newlines0; // position of all \n in the original code
+    };
+    std::vector<FileLocations> files;
+    // Location is continued for the imported files, i.e.,if the new file is
+    // imported then the location starts from the size of the previous file.
+    // For example: file_ends = [120, 200, 350]
+    std::vector<uint32_t> file_ends; // position of all ends of files
+    // For a given Location, we use the `file_ends` and `bisection` to determine
+    // the file (files' index), which the location is from. Then we use this index into
+    // the `files` vector and use `in_newlines` and other information to
+    // determine the line and column inside this file, and the `in_filename`
+    // field to determine the filename. This happens when the diagnostic is
+    // printed.
 
     // Converts a position in the output code to a position in the original code
     // Every character in the output code has a corresponding location in the
     // original code, so this function always succeeds
     uint32_t output_to_input_pos(uint32_t out_pos, bool show_last) const {
-        if (out_start.size() == 0) return 0;
-        uint32_t interval = bisection(out_start, out_pos)-1;
-        uint32_t rel_pos = out_pos - out_start[interval];
-        uint32_t in_pos = in_start[interval] + rel_pos;
-        if (preprocessor) {
+        // Determine where the error is from using position, i.e., loc
+        uint32_t index = bisection(file_ends, out_pos);
+        if (index == file_ends.size()) index -= 1;
+        if (files[index].out_start.size() == 0) return 0;
+        uint32_t interval = bisection(files[index].out_start, out_pos)-1;
+        uint32_t rel_pos = out_pos - files[index].out_start[interval];
+        uint32_t in_pos = files[index].in_start[interval] + rel_pos;
+        if (files[index].preprocessor) {
             // If preprocessor was used, do one more remapping
-            uint32_t interval0 = bisection(out_start0, in_pos)-1;
-            if (interval_type0[interval0] == 0) {
+            uint32_t interval0 = bisection(files[index].out_start0, in_pos)-1;
+            if (files[index].interval_type0[interval0] == 0) {
                 // 1:1 interval
-                uint32_t rel_pos0 = in_pos - out_start0[interval0];
-                uint32_t in_pos0 = in_start0[interval0] + rel_pos0;
+                uint32_t rel_pos0 = in_pos - files[index].out_start0[interval0];
+                uint32_t in_pos0 = files[index].in_start0[interval0] + rel_pos0;
                 return in_pos0;
             } else {
                 // many to many interval
                 uint32_t in_pos0;
-                if (in_pos == out_start0[interval0+1]-1 || show_last) {
+                if (in_pos == files[index].out_start0[interval0+1]-1 || show_last) {
                     // The end of the interval in "out" code
                     // Return the end of the interval in "in" code
-                    in_pos0 = in_start0[interval0]+in_size0[interval0]-1;
+                    in_pos0 = files[index].in_start0[interval0]+files[index].in_size0[interval0]-1;
                 } else {
                     // Otherwise return the beginning of the interval in "in"
-                    in_pos0 = in_start0[interval0];
+                    in_pos0 = files[index].in_start0[interval0];
                 }
                 return in_pos0;
             }
@@ -150,12 +164,19 @@ struct LocationManager {
     // `position` starts from 0
     // `line` and `col` starts from 1
     // `in_newlines` starts from 0
-    void pos_to_linecol(uint32_t position, uint32_t &line, uint32_t &col) const {
+    void pos_to_linecol(uint32_t position, uint32_t &line, uint32_t &col,
+            std::string &filename) const {
+        // Determine where the error is from using position, i.e., loc
+        uint32_t index = bisection(file_ends, position);
+        if (index == file_ends.size()) index -= 1;
+        filename = files[index].in_filename;
+        // Get the actual location by subtracting the previous file size.
+        if (index > 0) position -= file_ends[index - 1];
         const std::vector<uint32_t> *newlines;
-        if (preprocessor) {
-            newlines = &in_newlines0;
+        if (files[index].preprocessor) {
+            newlines = &files[index].in_newlines0;
         } else {
-            newlines = &in_newlines;
+            newlines = &files[index].in_newlines;
         }
         int32_t interval = bisection(*newlines, position);
         if (interval >= 1 && position == (*newlines)[interval-1]) {
@@ -179,9 +200,9 @@ struct LocationManager {
 
     void init_simple(const std::string &input) {
         uint32_t n = input.size();
-        out_start = {0, n};
-        in_start = {0, n};
-        get_newlines(input, in_newlines);
+        files.back().out_start = {0, n};
+        files.back().in_start = {0, n};
+        get_newlines(input, files.back().in_newlines);
     }
 
 };

src/libasr/location.h

@@ -11,6 +11,7 @@ namespace LFortran::LPython {
               uint32_t first_column;
               uint32_t last_line;
               uint32_t last_column;
+              std::string filename;
               uint32_t severity;
        };
        struct document_symbols {
@@ -19,6 +20,7 @@ namespace LFortran::LPython {
               uint32_t first_column;
               uint32_t last_line;
               uint32_t last_column;
+              std::string filename;
        };
 
 } // namespace LFortran::Python

src/libasr/lsp_interface.h

@@ -15,11 +15,11 @@
 namespace LFortran {
 
 Result<LPython::AST::Module_t*> parse(Allocator &al, const std::string &s,
-        diag::Diagnostics &diagnostics)
+        uint32_t prev_loc, diag::Diagnostics &diagnostics)
 {
     Parser p(al, diagnostics);
     try {
-        p.parse(s);
+        p.parse(s, prev_loc);
     } catch (const parser_local::TokenizerError &e) {
         Error error;
         diagnostics.diagnostics.push_back(e.d);
@@ -42,15 +42,15 @@ Result<LPython::AST::Module_t*> parse(Allocator &al, const std::string &s,
         p.result.p, p.result.size(), p.type_ignore.p, p.type_ignore.size());
 }
 
-void Parser::parse(const std::string &input)
+void Parser::parse(const std::string &input, uint32_t prev_loc)
 {
     inp = input;
     if (inp.size() > 0) {
         if (inp[inp.size()-1] != '\n') inp.append("\n");
     } else {
         inp.append("\n");
     }
-    m_tokenizer.set_string(inp);
+    m_tokenizer.set_string(inp, prev_loc);
     if (yyparse(*this) == 0) {
         return;
     }
@@ -116,13 +116,14 @@ Result<LPython::AST::ast_t*> parse_python_file(Allocator &al,
         const std::string &/*runtime_library_dir*/,
         const std::string &infile,
         diag::Diagnostics &diagnostics,
+        uint32_t prev_loc,
         bool new_parser) {
     LPython::AST::ast_t* ast;
     // We will be using the new parser from now on
     new_parser = true;
     LFORTRAN_ASSERT(new_parser)
     std::string input = read_file(infile);
-    Result<LPython::AST::Module_t*> res = parse(al, input, diagnostics);
+    Result<LPython::AST::Module_t*> res = parse(al, input, prev_loc, diagnostics);
     if (res.ok) {
         ast = (LPython::AST::ast_t*)res.result;
     } else {