Skip to content

Extend Location manger to handle multi-file import errors #1297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -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/*
Expand All @@ -72,6 +72,7 @@ integration_tests/b5/*
integration_tests/b6/*
integration_tests/_lpython-tmp-test-*
inst/bin/*
*.tmp
*.tlog
*.filters
*.obj
Expand Down
366 changes: 230 additions & 136 deletions src/bin/lpython.cpp

Large diffs are not rendered by default.

11 changes: 8 additions & 3 deletions src/libasr/codegen/asr_to_llvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
50 changes: 31 additions & 19 deletions src/libasr/diagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently, use this to fix the \r issue in the error_rendering.

out << blue_bold << std::setw(line_num_width)
<< std::to_string(s0.first_line) << " |" << reset << " "
<< line << std::endl;
Expand All @@ -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;
Expand All @@ -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;
Expand Down
11 changes: 4 additions & 7 deletions src/libasr/diagnostics.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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.
*
Expand Down
91 changes: 56 additions & 35 deletions src/libasr/location.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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]) {
Expand All @@ -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);
}

};
Expand Down
2 changes: 2 additions & 0 deletions src/libasr/lsp_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
11 changes: 6 additions & 5 deletions src/lpython/parser/parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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 {
Expand Down
Loading