Diagnostic messages: Fixing bugs and improving printing

[certik]

Now when we can test error/warning/hint messages (see tests/errors), we should work on improving how they are printed, and if we need any more information stored in the Diagnostics class, we can do that too.

Some bugs to fix:

• Implement class #391 (comment)

Urgent improvements:

• When two error labels are on the same line, we have to draw an ascii-art from the underline (both on the same line) to the label (on two separate lines)
• When error labels point to different files, we draw both files, make sure everything works with multiple files
• Error labels in a single file should be merged, and lines sorted (currently a later line in a file can be printed first)
• Add tests for these errors that need improving into tests/errors
• Add more tests into tests/errors for errors to sample a wide variety of error messages, right now we have 9, I am thinking at least 30 to 50. Test tokenizer, parser, semantics (I can't think of a codegen error that is either not temporary or a bug in semantics, so we don't need to test those) --- then once we change how errors are printed, we can see the impact (just run ./run_tests.py -u) to make sure all corner cases still look good:
  • Tokenizer errors (one or two)
  • Parser errors (a few)
  • Semantic errors (a lot)

Once these are fixed and tests added, we should work on good error messages, for example when you call a function f(1, 2) and some of the arguments don't agree, to highlight the declaration (possibly in different file), show where it was imported in the current file, show the expected type, show the current type, and so on.

Other improvements:

• Optionally using unicode box drawing characters
  • Example: Test error messages #412 (comment)
  • This should probably be the default, including in our tests for errors, but a command line option --no-unicode-errors will replace unicode characters with just ascii characters.
• Optionally experiment with highlighting the text itself (in red or blue, possibly in the background) instead of using the ~~~ and ---- underlines, this should save vertical space. This must be optional, as you need to be able to see colors. With --no-color we should still print underlines.
• We can also experiment with some nice html output, possibly printing the source code (syntax highlighted) and then highlighting errors/warnings/hints somehow, and when you hover over it with a mouse, show more information.

Why to improve error messages

Already the diagnostic messages in LPython/LFortran are very good, but we have the opportunity to push this to (almost) perfection, we have the infrastructure, we have the ideas (see above), so we should just do it. Every user will sooner or later see these messages, and if they are beautiful, clear, to the point, it's a pleasure to see, and you want to see more of them: it's an art, it is like a nice painting that the compiler presents to the user, on top of their code. It will be one reason to use our compilers, to get these messages, because it makes the process of developing the code a pleasure.

[namannimmo10]

Another one to fix:

% lpython --show-asr integration_tests/test_builtin_pow.py --indent
Internal Compiler Error: Unhandled exception
Traceback (most recent call last):
  File "/Users/namannimmo/Applications/oss/lpython/src/bin/lpython.cpp", line 755
    return emit_asr(arg_file, runtime_library_dir,
  File "/Users/namannimmo/Applications/oss/lpython/src/bin/lpython.cpp", line 180
    std::cout << LFortran::pickle(*asr, compiler_options.use_colors, compiler_options.indent,
  File "/Users/namannimmo/Applications/oss/lpython/src/lpython/pickle.cpp", line 90
    return pickle((ASR::asr_t &)asr, colors, indent, show_intrinsic_modules);
  File "/Users/namannimmo/Applications/oss/lpython/src/lpython/pickle.cpp", line 84
    v.show_intrinsic_modules = show_intrinsic_modules;
  File "/Users/namannimmo/Applications/oss/lpython/src/libasr/asr.h", line 2430
    void visit_asr(const asr_t &b) { visit_asr_t(b, self()); }
  File "/Users/namannimmo/Applications/oss/lpython/src/libasr/asr.h", line 2408
    case asrType::unit: { v.visit_unit((const unit_t &)x); return; }
  File "/Users/namannimmo/Applications/oss/lpython/src/libasr/asr.h", line 2431
    void visit_unit(const unit_t &b) { visit_unit_t(b, self()); }
  File "/Users/namannimmo/Applications/oss/lpython/src/libasr/asr.h", line 2263
    switch (x.type) {
  File "/Users/namannimmo/Applications/oss/lpython/src/libasr/asr.h", line 7600
    for(int times = 0; times < tmp2; times++)
  File "/Users/namannimmo/Applications/oss/lpython/src/libasr/asr.h", line 7537
    LFORTRAN_ASSERT(indent_level >= 0);
AssertFailed: indent_level >= 0

FYI, this generates the ASR without --indent.

[Thirumalai-Shaktivel]

Thanks for pointing it out, @namannimmo10!
This issue is related to --indent which was also reported here! Yup, we need to fix this...

[certik]

Another one: instead of this:

4 |     x: i32[4] = [1, 2, 3, 4]
  |     ^           ^^^^^^^^^^^^ type mismatch (i32[4] and list[i32])

do this:

4 |     x: i32[4] = [1, 2, 3, 4]
  |     ^           ^^^^^^^^^^^^
  |     +-- i32[4]           |
  |                          +---- list[i32]

or even

4 |     x: i32[4] = [1, 2, 3, 4]
  |     ^           ^^^^^^^^^^^^ list[i32]
  |     +-- i32[4]           
  |                         

or

4 |     x: i32[4] = [1, 2, 3, 4]
  |     ^           ^^^^^^^^^^^^ list[i32]
  |    i32[4]           
  |                         

[certik]

Here is another one: #412 (comment)

[Smit-create]

4 |     x: i32[4] = [1, 2, 3, 4]
  |     ^           ^^^^^^^^^^^^
  |     +-- i32[4]           |
  |                          +---- list[i32]

Does this design seems general? because this is looking specifically to the assignment. Moreover, looking into this, I see that we might also need to pass additional information in semantic errors to show this kind of error.

[certik]

Yes, the design is general (almost any message can be encoded), but the visualization of the diagnostic messages is not.