Extend LocationManager to handle multiple files

[certik]

Currently we can convert the linear location information (start/end) to a file + line + column number, but it only works for one file. We need to extend this mechanism to multiple files (since a Python program typically imports multiple files).

To do that, the LocationManager simply "concatenates" all the files, and then the linear index is pointing into this concatenated stream. So we store a list of start indices for each file, then bisect it to obtain the file (filename) and then within this file we remap as we do now to obtain line + column numbers.

[ubaidsk]

I am looking at this issue for some time/days. I would like to work on this. Shall I give it a try @Thirumalai-Shaktivel ?

[Thirumalai-Shaktivel]

I haven't started working on this issue.

I would like to work on this. Shall I give it a try @Thirumalai-Shaktivel ?

Sure, that would be very helpful.

[ubaidsk]

Thank you!

[ubaidsk]

I have the following doubts, please possibly clarify.

lpython/src/libasr/location.h

Lines 97 to 98 in d4a6d53

	std::vector<uint32_t> out_start; // consecutive intervals in the output code
	std::vector<uint32_t> in_start; // start + size in the original code

1. What is the difference between output code and original code? Why could we not directly use the original code for storing positions?

lpython/src/libasr/location.h

Lines 182 to 183 in d4a6d53

	out_start = {0, n};
	in_start = {0, n};

2. Where do we populate the values for the out_start and in_start variables?
3. Also, could someone possibly break down this issue into smaller sub-parts or share some hints about solving this, please?

[Thirumalai-Shaktivel]

After thinking more about this issue now, I think instead of concatenating all the files, ...
Why not simply print the error message in compile_module_till_asr() (since all the imported files are compiled till asr), along with the current file Semantic error:

lpython/src/lpython/semantics/python_ast_to_asr.cpp

Lines 242 to 277 in f9adfe3

	ASR::TranslationUnit_t* compile_module_till_asr(Allocator& al,
	std::vector<std::string> &rl_path, std::string infile,
	const Location &loc,
	const std::function<void (const std::string &, const Location &)> err,
	bool allow_implicit_casting) {
	// TODO: diagnostic should be an argument to this function
	diag::Diagnostics diagnostics;
	Result<AST::ast_t*> r = parse_python_file(al, rl_path[0], infile,
	diagnostics, false);
	if (!r.ok) {
	err("The file '" + infile + "' failed to parse", loc);
	}
	LFortran::LPython::AST::ast_t* ast = r.result;
	// Convert the module from AST to ASR
	LFortran::LocationManager lm;
	lm.in_filename = infile;
	LFortran::CompilerOptions compiler_options;
	compiler_options.disable_main = true;
	compiler_options.symtab_only = false;
	Result<ASR::TranslationUnit_t*> r2 = python_ast_to_asr(al, *ast,
	diagnostics, compiler_options, false, infile, allow_implicit_casting);
	// TODO: Uncomment once a check is added for ensuring
	// that module.py file hasn't changed between
	// builds.
	// save_pyc_files(*r2.result, infile + "c");
	std::string input;
	read_file(infile, input);
	std::cerr << diagnostics.render(input, lm, compiler_options);
	if (!r2.ok) {
	LFORTRAN_ASSERT(diagnostics.has_error())
	return nullptr; // Error
	}
	return r2.result;
	}

The error message would look something like this:

semantic error: Type mismatch in annotation-assignment, the types must be compatible
 --> /home/thirumalai/Open_Source/lpython/src/bin/../runtime/expr.py:3:1
  |
3 | e: i32 = 1.1
  | ^        ^^^ type mismatch ('i32' and 'f64')


Note: if any of the above error or warning messages are not clear or are lacking
context please report it to us (we consider that a bug that needs to be fixed).

semantic error: The file '/home/thirumalai/Open_Source/lpython/src/bin/../runtime/expr.py' failed due to semantic error
 --> examples/expr2.py:1:1
  |
1 | from expr import e
  | ^^^^^^^^^^^^^^^^^^ 


Note: if any of the above error or warning messages are not clear or are lacking
context please report it to us (we consider that a bug that needs to be fixed).

@certik @Shaikh-Ubaid What do you think?

[ubaidsk]

I am working on this issue. I probably need some more time, please.

[ubaidsk]

Also, I came across this

Code:

def main0():
    x: i32
    x = 3.14
    y:i32
    y = 2.1
    z: i32
    z = -3.0
    print(x)
    print(y)
    print(z)

main0()

Output:

(lp) ubaid@ubaid-Lenovo-ideapad-330-15ARR:~/OpenSource/lpython$ lpython examples/expr2.py
semantic error: Type mismatch in assignment, the types must be compatible
 --> examples/expr2.py:4:5
  |
4 |     x = 3.14
  |     ^   ^^^^ type mismatch ('i32' and 'f64')


Note: if any of the above error or warning messages are not clear or are lacking
context please report it to us (we consider that a bug that needs to be fixed). 

During our last meet, Ondrej discussed that lpython (like other compiler's usually do) should detect (and print) all the compilation errors in all the files (the main module file + other imported files). In the above example, we see that there are three semantic (type mismatch) errors, but lpython at present detects only one. Please, could someone possibly share if this is desirable or if it is a bug or a feature yet to implemented/supported?

CC: @certik

[ubaidsk]

To do that, the LocationManager simply "concatenates" all the files
Please, could someone possibly share if this is desirable or if it is a bug or a feature yet to implemented/supported?

With respect to the above comment, at present, it seems that even if the LocationManager concatenates all the files, the compilation would stop at the first error captured (in the main module or in one of the imported files).

[Thirumalai-Shaktivel]

#1218 (comment)
This feature would be very useful, and yes, this is yet and has to be implemented.

But, I think here @certik is talking about something like the following. Consider an example:
a.py

i: i32 = 3.142

b.py

from a import i
j: i32 = i

c.py

from b import j
print(j)

So, here we have to push_back or concatenate all the filenames to a vector(with location, I think).

_	_
_	_
a.py	1 <--
b.py	1
c.py	1
stack	loc

From the above example, there is an error in a.py. Now, Print the error message along with the other file pointing to it.
It would look like this:

semantic error: Type mismatch in assignment, the types must be compatible
 --> a.py:4:5
  |
1 |     i: i32 = 3.142
  |     ^        ^^^^^ type mismatch ('i32' and 'f64')
Imported here:
 --> b.py:1:1
  |
1 | from a import i
  | ^^^^^^^^^^^^^^^
Imported here:
 --> c.py:1:1
  |
1 | from b import j
  | ^^^^^^^^^^^^^^^ 

[ubaidsk]

Thank you for the clarification, Thirumalai. It seems the situation you shared here #1218 (comment) is (to some extent) partially supported by the current lpython. I attempted to (hopefully) improve it in #1294, but it did not need to modify the LocationManager to support it.

I am slightly confused about the aim of this issue. This issue is titled Extend LocationManager to handle multiple files, indicating we should/would be needing to modify the LocationManager (since we need to extend it). At the same time, if we attempt to support the situation in #1218 (comment), it seems we could support it without modifying the LocationManager.